From e7e64bf39abb90cce9df2f531873d316f5d6b18f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vicent=20Mart=C3=AD?= Date: Mon, 4 Dec 2017 19:20:38 +0100 Subject: [PATCH] compiler: Add error output to the compiler (#3935) --- script/add-grammar | 37 +++-- script/grammar-compiler | 2 +- tools/grammars/Gopkg.lock | 8 +- tools/grammars/Gopkg.toml | 4 + tools/grammars/cmd/grammar-compiler/main.go | 164 ++++++++++++-------- tools/grammars/compiler/converter.go | 54 +++++-- tools/grammars/compiler/loader.go | 2 +- tools/grammars/compiler/walker.go | 4 +- 8 files changed, 185 insertions(+), 90 deletions(-) diff --git a/script/add-grammar b/script/add-grammar index 1a3de737..dc9cd6e7 100755 --- a/script/add-grammar +++ b/script/add-grammar @@ -1,6 +1,7 @@ #!/usr/bin/env ruby require "optparse" +require "open3" ROOT = File.expand_path("../../", __FILE__) @@ -42,6 +43,17 @@ def log(msg) puts msg if $verbose end +def command(*args) + log "$ #{args.join(' ')}" + output, status = Open3.capture2e(*args) + if !status.success? + output.each_line do |line| + log " > #{line}" + end + warn "Command failed. Aborting." + exit 1 + end +end usage = """Usage: #{$0} [-v|--verbose] [--replace grammar] url @@ -51,12 +63,12 @@ Examples: """ $replace = nil -$verbose = false +$verbose = true OptionParser.new do |opts| opts.banner = usage - opts.on("-v", "--verbose", "Print verbose feedback to STDOUT") do - $verbose = true + opts.on("-q", "--quiet", "Do not print output unless there's a failure") do + $verbose = false end opts.on("-rSUBMODULE", "--replace=SUBMODDULE", "Replace an existing grammar submodule.") do |name| $replace = name @@ -82,23 +94,22 @@ Dir.chdir(ROOT) if repo_old log "Deregistering: #{repo_old}" - `git submodule deinit #{repo_old}` - `git rm -rf #{repo_old}` - `script/grammar-compiler -update` + command('git', 'submodule', 'deinit', repo_old) + command('git', 'rm', '-rf', repo_old) + command('script/grammar-compiler', 'update', '-f') end log "Registering new submodule: #{repo_new}" -`git submodule add -f #{https} #{repo_new}` -exit 1 if $?.exitstatus > 0 -`script/grammar-compiler -add #{repo_new}` +command('git', 'submodule', 'add', '-f', https, repo_new) +command('script/grammar-compiler', 'add', repo_new) log "Confirming license" if repo_old - `script/licensed` + command('script/licensed') else - `script/licensed --module "#{repo_new}"` + command('script/licensed', '--module', repo_new) end log "Updating grammar documentation in vendor/README.md" -`bundle exec rake samples` -`script/list-grammars` +command('bundle', 'exec', 'rake', 'samples') +command('script/list-grammars') diff --git a/script/grammar-compiler b/script/grammar-compiler index bded3f8f..59a66e3d 100755 --- a/script/grammar-compiler +++ b/script/grammar-compiler @@ -9,4 +9,4 @@ mkdir -p grammars exec docker run --rm \ -u $(id -u $USER):$(id -g $USER) \ -v $PWD:/src/linguist \ - -w /src/linguist -ti $image "$@" + -w /src/linguist $image "$@" diff --git a/tools/grammars/Gopkg.lock b/tools/grammars/Gopkg.lock index 2624c57b..b0328e04 100644 --- a/tools/grammars/Gopkg.lock +++ b/tools/grammars/Gopkg.lock @@ -25,6 +25,12 @@ packages = ["."] revision = "06020f85339e21b2478f756a78e295255ffa4d6a" +[[projects]] + name = "github.com/urfave/cli" + packages = ["."] + revision = "cfb38830724cc34fedffe9a2a29fb54fa9169cd1" + version = "v1.20.0" + [[projects]] name = "gopkg.in/cheggaaa/pb.v1" packages = ["."] @@ -40,6 +46,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "eb10157687c05a542025c119a5280abe429e29141bde70dd437d48668f181861" + inputs-digest = "ba2e3150d728692b49e3e2d652b6ea23db82777c340e0c432cd4af6f0eef9f55" solver-name = "gps-cdcl" solver-version = 1 diff --git a/tools/grammars/Gopkg.toml b/tools/grammars/Gopkg.toml index 3e5f4979..3583529d 100644 --- a/tools/grammars/Gopkg.toml +++ b/tools/grammars/Gopkg.toml @@ -17,3 +17,7 @@ [[constraint]] name = "gopkg.in/cheggaaa/pb.v1" version = "1.0.18" + +[[constraint]] + name = "github.com/urfave/cli" + version = "1.20.0" diff --git a/tools/grammars/cmd/grammar-compiler/main.go b/tools/grammars/cmd/grammar-compiler/main.go index 62485922..7145a5cd 100644 --- a/tools/grammars/cmd/grammar-compiler/main.go +++ b/tools/grammars/cmd/grammar-compiler/main.go @@ -1,80 +1,120 @@ package main import ( - "flag" - "fmt" "os" - "os/exec" "github.com/github/linguist/tools/grammars/compiler" + "github.com/urfave/cli" ) -var linguistRoot = flag.String("linguist", "", "path to Linguist installation") -var protoOut = flag.String("proto", "", "dump Protobuf library") -var jsonOut = flag.String("json", "", "dump JSON output") -var addGrammar = flag.String("add", "", "add a new grammar source") -var updateList = flag.Bool("update", false, "update grammars.yml instead of verifying its contents") -var report = flag.String("report", "", "write report to file") +func cwd() string { + cwd, _ := os.Getwd() + return cwd +} -func fatal(err error) { - fmt.Fprintf(os.Stderr, "FATAL: %s\n", err) - os.Exit(1) +func wrap(err error) error { + return cli.NewExitError(err, 255) } func main() { - flag.Parse() + app := cli.NewApp() + app.Name = "Linguist Grammars Compiler" + app.Usage = "Compile user-submitted grammars and check them for errors" - if _, err := exec.LookPath("csonc"); err != nil { - fatal(err) + app.Flags = []cli.Flag{ + cli.StringFlag{ + Name: "linguist-path", + Value: cwd(), + Usage: "path to Linguist root", + }, } - if *linguistRoot == "" { - cwd, err := os.Getwd() - if err != nil { - fatal(err) - } - *linguistRoot = cwd + app.Commands = []cli.Command{ + { + Name: "add", + Usage: "add a new grammar source", + Flags: []cli.Flag{ + cli.BoolFlag{ + Name: "force, f", + Usage: "ignore compilation errors", + }, + }, + Action: func(c *cli.Context) error { + conv, err := compiler.NewConverter(c.String("linguist-path")) + if err != nil { + return wrap(err) + } + if err := conv.AddGrammar(c.Args().First()); err != nil { + if !c.Bool("force") { + return wrap(err) + } + } + if err := conv.WriteGrammarList(); err != nil { + return wrap(err) + } + return nil + }, + }, + { + Name: "update", + Usage: "update grammars.yml with the contents of the grammars library", + Flags: []cli.Flag{ + cli.BoolFlag{ + Name: "force, f", + Usage: "write grammars.yml even if grammars fail to compile", + }, + }, + Action: func(c *cli.Context) error { + conv, err := compiler.NewConverter(c.String("linguist-path")) + if err != nil { + return wrap(err) + } + if err := conv.ConvertGrammars(true); err != nil { + return wrap(err) + } + if err := conv.Report(); err != nil { + if !c.Bool("force") { + return wrap(err) + } + } + if err := conv.WriteGrammarList(); err != nil { + return wrap(err) + } + return nil + }, + }, + { + Name: "compile", + Usage: "convert the grammars from the library", + Flags: []cli.Flag{ + cli.StringFlag{Name: "proto-out, P"}, + cli.StringFlag{Name: "out, o"}, + }, + Action: func(c *cli.Context) error { + conv, err := compiler.NewConverter(c.String("linguist-path")) + if err != nil { + return cli.NewExitError(err, 1) + } + if err := conv.ConvertGrammars(false); err != nil { + return cli.NewExitError(err, 1) + } + if out := c.String("proto-out"); out != "" { + if err := conv.WriteProto(out); err != nil { + return cli.NewExitError(err, 1) + } + } + if out := c.String("out"); out != "" { + if err := conv.WriteJSON(out); err != nil { + return cli.NewExitError(err, 1) + } + } + if err := conv.Report(); err != nil { + return wrap(err) + } + return nil + }, + }, } - conv, err := compiler.NewConverter(*linguistRoot) - if err != nil { - fatal(err) - } - - if *addGrammar != "" { - if err := conv.AddGrammar(*addGrammar); err != nil { - fatal(err) - } - } - - if err := conv.ConvertGrammars(*updateList); err != nil { - fatal(err) - } - - if err := conv.WriteGrammarList(); err != nil { - fatal(err) - } - - if *protoOut != "" { - if err := conv.WriteProto(*protoOut); err != nil { - fatal(err) - } - } - - if *jsonOut != "" { - if err := conv.WriteJSON(*jsonOut); err != nil { - fatal(err) - } - } - - if *report == "" { - conv.Report(os.Stderr) - } else { - f, err := os.Create(*report) - if err != nil { - fatal(err) - } - conv.Report(f) - f.Close() - } + app.Run(os.Args) } diff --git a/tools/grammars/compiler/converter.go b/tools/grammars/compiler/converter.go index a557301a..aeadaefa 100644 --- a/tools/grammars/compiler/converter.go +++ b/tools/grammars/compiler/converter.go @@ -3,7 +3,6 @@ package compiler import ( "encoding/json" "fmt" - "io" "io/ioutil" "os" "path" @@ -52,6 +51,16 @@ func (conv *Converter) work() { conv.wg.Done() } +func (conv *Converter) tmpScopes() map[string]bool { + scopes := make(map[string]bool) + for _, ary := range conv.grammars { + for _, s := range ary { + scopes[s] = true + } + } + return scopes +} + func (conv *Converter) AddGrammar(source string) error { repo := conv.Load(source) if len(repo.Files) == 0 { @@ -61,17 +70,30 @@ func (conv *Converter) AddGrammar(source string) error { conv.grammars[source] = repo.Scopes() conv.modified = true + knownScopes := conv.tmpScopes() + repo.FixRules(knownScopes) + + if len(repo.Errors) > 0 { + fmt.Fprintf(os.Stderr, "The new grammar %s contains %d errors:\n", + repo, len(repo.Errors)) + for _, err := range repo.Errors { + fmt.Fprintf(os.Stderr, " - %s\n", err) + } + fmt.Fprintf(os.Stderr, "\n") + return fmt.Errorf("failed to compile the given grammar") + } + fmt.Printf("OK! added grammar source '%s'\n", source) for scope := range repo.Files { fmt.Printf("\tnew scope: %s\n", scope) } - return nil } -func (conv *Converter) ScopeMap() map[string]*Repository { +func (conv *Converter) AllScopes() map[string]bool { + // Map from scope -> Repository first to error check + // possible duplicates allScopes := make(map[string]*Repository) - for _, repo := range conv.Loaded { for scope := range repo.Files { if original := allScopes[scope]; original != nil { @@ -82,7 +104,12 @@ func (conv *Converter) ScopeMap() map[string]*Repository { } } - return allScopes + // Convert to scope -> bool + scopes := make(map[string]bool) + for s := range allScopes { + scopes[s] = true + } + return scopes } func (conv *Converter) ConvertGrammars(update bool) error { @@ -112,7 +139,7 @@ func (conv *Converter) ConvertGrammars(update bool) error { conv.modified = true } - knownScopes := conv.ScopeMap() + knownScopes := conv.AllScopes() for source, repo := range conv.Loaded { repo.FixRules(knownScopes) @@ -190,7 +217,7 @@ func (conv *Converter) WriteGrammarList() error { return ioutil.WriteFile(ymlpath, outyml, 0666) } -func (conv *Converter) Report(w io.Writer) { +func (conv *Converter) Report() error { var failed []*Repository for _, repo := range conv.Loaded { if len(repo.Errors) > 0 { @@ -202,13 +229,20 @@ func (conv *Converter) Report(w io.Writer) { return failed[i].Source < failed[j].Source }) + total := 0 for _, repo := range failed { - fmt.Fprintf(w, "- [ ] %s (%d errors)\n", repo, len(repo.Errors)) + fmt.Fprintf(os.Stderr, "- [ ] %s (%d errors)\n", repo, len(repo.Errors)) for _, err := range repo.Errors { - fmt.Fprintf(w, " - [ ] %s\n", err) + fmt.Fprintf(os.Stderr, " - [ ] %s\n", err) } - fmt.Fprintf(w, "\n") + fmt.Fprintf(os.Stderr, "\n") + total += len(repo.Errors) } + + if total > 0 { + return fmt.Errorf("the grammar library contains %d errors", total) + } + return nil } func NewConverter(root string) (*Converter, error) { diff --git a/tools/grammars/compiler/loader.go b/tools/grammars/compiler/loader.go index 6820364b..b1770092 100644 --- a/tools/grammars/compiler/loader.go +++ b/tools/grammars/compiler/loader.go @@ -81,7 +81,7 @@ func (repo *Repository) CompareScopes(scopes []string) { } } -func (repo *Repository) FixRules(knownScopes map[string]*Repository) { +func (repo *Repository) FixRules(knownScopes map[string]bool) { for _, file := range repo.Files { w := walker{ File: file, diff --git a/tools/grammars/compiler/walker.go b/tools/grammars/compiler/walker.go index 3132736e..c66f0813 100644 --- a/tools/grammars/compiler/walker.go +++ b/tools/grammars/compiler/walker.go @@ -19,7 +19,7 @@ func (w *walker) checkInclude(rule *grammar.Rule) { } include = strings.Split(include, "#")[0] - _, ok := w.Known[include] + ok := w.Known[include] if !ok { if !w.Missing[include] { w.Missing[include] = true @@ -73,7 +73,7 @@ func (w *walker) walk(rule *grammar.Rule) { type walker struct { File *LoadedFile - Known map[string]*Repository + Known map[string]bool Missing map[string]bool Errors []error }