From 493007b50417ec4f655afe57ba7044b4b929f1cf Mon Sep 17 00:00:00 2001 From: Colin Seymour Date: Tue, 3 Aug 2021 15:55:36 +0100 Subject: [PATCH] Treat grammar warnings as non-fatal (#5497) * Switch to using Go modules and later Go version * Update cli module * Only show warning-like errors in verbose output * Split warnings when adding new grammar * Print last line to stderr too * Improve warning * Show new grammar warnings * Don't use variadic option * NewExitError is deprecated. Use Exit instead * Flip opt name * Update formatting and wording * Remove checkboxes --- script/add-grammar | 18 ++++-- tools/grammars/Dockerfile | 7 +-- tools/grammars/Gopkg.lock | 51 ----------------- tools/grammars/Gopkg.toml | 23 -------- tools/grammars/cmd/grammar-compiler/main.go | 46 ++++++++------- tools/grammars/compiler/converter.go | 62 +++++++++++++++++---- tools/grammars/go.mod | 16 ++++++ tools/grammars/go.sum | 42 ++++++++++++++ 8 files changed, 151 insertions(+), 114 deletions(-) delete mode 100644 tools/grammars/Gopkg.lock delete mode 100644 tools/grammars/Gopkg.toml create mode 100644 tools/grammars/go.mod create mode 100644 tools/grammars/go.sum diff --git a/script/add-grammar b/script/add-grammar index a27f30f551..75d168cb19 100755 --- a/script/add-grammar +++ b/script/add-grammar @@ -43,16 +43,24 @@ def log(msg) puts msg if $verbose end -def command(*args) +def command(*args, hide_warnings: false) log "$ #{args.join(' ')}" - output, status = Open3.capture2e(*args) - if !status.success? + stdout, stderr, status = Open3.capture3(*args) + unless status.success? + output = stdout.strip + "\n" + stderr.strip output.each_line do |line| log " > #{line}" end warn "Command failed. Aborting." exit 1 end + return if stderr.empty? + + unless hide_warnings + stderr.each_line do |line| + log " > #{line}" + end + end end usage = """Usage: @@ -100,7 +108,7 @@ if repo_old log "Deregistering: #{repo_old}" command('git', 'submodule', 'deinit', repo_old) command('git', 'rm', '-rf', repo_old) - command('script/grammar-compiler', 'update', '-f') + command('script/grammar-compiler', 'update', '-f', hide_warnings: true) end log "Registering new submodule: #{repo_new}" @@ -111,6 +119,6 @@ log "Caching grammar license" command("bundle", "exec", "licensed", "cache", "-c", "vendor/licenses/config.yml") log "Updating grammar documentation in vendor/README.md" -command('bundle', 'exec', 'rake', 'samples') +command('bundle', 'exec', 'rake', 'samples', hide_warnings: true) command('script/sort-submodules') command('script/list-grammars') diff --git a/tools/grammars/Dockerfile b/tools/grammars/Dockerfile index 09b5863810..e93951b620 100644 --- a/tools/grammars/Dockerfile +++ b/tools/grammars/Dockerfile @@ -1,10 +1,10 @@ -FROM golang:1.9.2 +FROM golang:1.16 WORKDIR /go/src/github.com/github/linguist/tools/grammars RUN curl -sL https://deb.nodesource.com/setup_6.x | bash - && \ apt-get update && \ - apt-get install -y nodejs cmake && \ + apt-get install -y nodejs cmake npm && \ npm install -g season && \ cd /tmp && git clone https://github.com/vmg/pcre && \ mkdir -p /tmp/pcre/build && cd /tmp/pcre/build && \ @@ -21,10 +21,9 @@ RUN curl -sL https://deb.nodesource.com/setup_6.x | bash - && \ -G "Unix Makefiles" && \ make && make install && \ rm -rf /tmp/pcre && \ - cd /go && go get -u github.com/golang/dep/cmd/dep && \ rm -rf /var/lib/apt/lists/* COPY . . -RUN dep ensure && go install ./cmd/grammar-compiler +RUN go install ./cmd/grammar-compiler ENTRYPOINT ["grammar-compiler"] diff --git a/tools/grammars/Gopkg.lock b/tools/grammars/Gopkg.lock deleted file mode 100644 index b0328e0427..0000000000 --- a/tools/grammars/Gopkg.lock +++ /dev/null @@ -1,51 +0,0 @@ -# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. - - -[[projects]] - branch = "master" - name = "github.com/golang/protobuf" - packages = ["proto"] - revision = "1e59b77b52bf8e4b449a57e6f79f21226d571845" - -[[projects]] - branch = "master" - name = "github.com/groob/plist" - packages = ["."] - revision = "7b367e0aa692e62a223e823f3288c0c00f519a36" - -[[projects]] - name = "github.com/mattn/go-runewidth" - packages = ["."] - revision = "9e777a8366cce605130a531d2cd6363d07ad7317" - version = "v0.0.2" - -[[projects]] - branch = "master" - name = "github.com/mitchellh/mapstructure" - packages = ["."] - revision = "06020f85339e21b2478f756a78e295255ffa4d6a" - -[[projects]] - name = "github.com/urfave/cli" - packages = ["."] - revision = "cfb38830724cc34fedffe9a2a29fb54fa9169cd1" - version = "v1.20.0" - -[[projects]] - name = "gopkg.in/cheggaaa/pb.v1" - packages = ["."] - revision = "657164d0228d6bebe316fdf725c69f131a50fb10" - version = "v1.0.18" - -[[projects]] - branch = "v2" - name = "gopkg.in/yaml.v2" - packages = ["."] - revision = "287cf08546ab5e7e37d55a84f7ed3fd1db036de5" - -[solve-meta] - analyzer-name = "dep" - analyzer-version = 1 - inputs-digest = "ba2e3150d728692b49e3e2d652b6ea23db82777c340e0c432cd4af6f0eef9f55" - solver-name = "gps-cdcl" - solver-version = 1 diff --git a/tools/grammars/Gopkg.toml b/tools/grammars/Gopkg.toml deleted file mode 100644 index 3583529d44..0000000000 --- a/tools/grammars/Gopkg.toml +++ /dev/null @@ -1,23 +0,0 @@ -[[constraint]] - branch = "v2" - name = "gopkg.in/yaml.v2" - -[[constraint]] - branch = "master" - name = "github.com/groob/plist" - -[[constraint]] - branch = "master" - name = "github.com/golang/protobuf" - -[[constraint]] - branch = "master" - name = "github.com/mitchellh/mapstructure" - -[[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 a09f5ffd79..c5881b68f8 100644 --- a/tools/grammars/cmd/grammar-compiler/main.go +++ b/tools/grammars/cmd/grammar-compiler/main.go @@ -4,7 +4,7 @@ import ( "os" "github.com/github/linguist/tools/grammars/compiler" - "github.com/urfave/cli" + "github.com/urfave/cli/v2" ) func cwd() string { @@ -13,7 +13,7 @@ func cwd() string { } func wrap(err error) error { - return cli.NewExitError(err, 255) + return cli.Exit(err, 255) } func main() { @@ -22,25 +22,26 @@ func main() { app.Usage = "Compile user-submitted grammars and check them for errors" app.Flags = []cli.Flag{ - cli.StringFlag{ + &cli.StringFlag{ Name: "linguist-path", Value: cwd(), Usage: "path to Linguist root", }, } - app.Commands = []cli.Command{ + app.Commands = []*cli.Command{ { Name: "add", Usage: "add a new grammar source", Flags: []cli.Flag{ - cli.BoolFlag{ - Name: "force, f", + &cli.BoolFlag{ + Name: "force", + Aliases: []string{"f"}, Usage: "ignore compilation errors", }, }, Action: func(c *cli.Context) error { - conv, err := compiler.NewConverter(c.GlobalString("linguist-path")) + conv, err := compiler.NewConverter(c.String("linguist-path")) if err != nil { return wrap(err) } @@ -59,20 +60,27 @@ func main() { Name: "update", Usage: "update grammars.yml with the contents of the grammars library", Flags: []cli.Flag{ - cli.BoolFlag{ - Name: "force, f", + &cli.BoolFlag{ + Name: "force", + Aliases: []string{"f"}, Usage: "write grammars.yml even if grammars fail to compile", }, + &cli.BoolFlag{ + Name: "verbose", + Aliases: []string{"v"}, + Usage: "show all warnings", + Value: false, + }, }, Action: func(c *cli.Context) error { - conv, err := compiler.NewConverter(c.GlobalString("linguist-path")) + 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 err := conv.Report(c.Bool("verbose")); err != nil { if !c.Bool("force") { return wrap(err) } @@ -87,28 +95,28 @@ func main() { Name: "compile", Usage: "convert the grammars from the library", Flags: []cli.Flag{ - cli.StringFlag{Name: "proto-out, P"}, - cli.StringFlag{Name: "out, o"}, + &cli.StringFlag{Name: "proto-out", Aliases: []string{"P"}}, + &cli.StringFlag{Name: "out", Aliases: []string{"o"}}, }, Action: func(c *cli.Context) error { - conv, err := compiler.NewConverter(c.GlobalString("linguist-path")) + conv, err := compiler.NewConverter(c.String("linguist-path")) if err != nil { - return cli.NewExitError(err, 1) + return cli.Exit(err, 1) } if err := conv.ConvertGrammars(false); err != nil { - return cli.NewExitError(err, 1) + return cli.Exit(err, 1) } if out := c.String("proto-out"); out != "" { if err := conv.WriteProto(out); err != nil { - return cli.NewExitError(err, 1) + return cli.Exit(err, 1) } } if out := c.String("out"); out != "" { if err := conv.WriteJSON(out); err != nil { - return cli.NewExitError(err, 1) + return cli.Exit(err, 1) } } - if err := conv.Report(); err != nil { + if err := conv.Report(false); err != nil { return wrap(err) } return nil diff --git a/tools/grammars/compiler/converter.go b/tools/grammars/compiler/converter.go index 23e7883b76..a8d31bcff8 100644 --- a/tools/grammars/compiler/converter.go +++ b/tools/grammars/compiler/converter.go @@ -75,13 +75,35 @@ func (conv *Converter) AddGrammar(source string) error { 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) + // Split out warnings from errors + warnings := []error{} + errors := []error{} + for _, e := range repo.Errors { + switch e.(type) { + case *MissingIncludeError, *UnknownKeysError: + warnings = append(warnings, e) + default: + errors = append(errors, e) + } + } + if len(errors) > 0 { + fmt.Fprintf(os.Stderr, "%d errors found in new grammar '%s':\n", + len(errors), repo) + for _, err := range errors { + fmt.Fprintf(os.Stderr, "- %s\n", err) + } + fmt.Fprintf(os.Stderr, "\n") + return fmt.Errorf("Compilation failed. Aborting") + } + if len(warnings) > 0 { + fmt.Fprintf(os.Stderr, "%d warnings found when compiling new grammar '%s':\n", + len(warnings), repo) + for _, err := range warnings { + fmt.Fprintf(os.Stderr, "- %s\n", err) + } + fmt.Fprintf(os.Stderr, "\n") + fmt.Fprintf(os.Stderr, "These warnings are not fatal, but may mean the syntax highlighting on GitHub.com may not be as expected.\n\n") } - fmt.Fprintf(os.Stderr, "\n") - return fmt.Errorf("failed to compile the given grammar") } fmt.Printf("OK! added grammar source '%s'\n", source) @@ -227,7 +249,7 @@ func (conv *Converter) WriteGrammarList() error { return ioutil.WriteFile(ymlpath, outyml, 0666) } -func (conv *Converter) Report() error { +func (conv *Converter) Report(verbose bool) error { var failed []*Repository for _, repo := range conv.Loaded { if len(repo.Errors) > 0 { @@ -241,16 +263,32 @@ func (conv *Converter) Report() error { total := 0 for _, repo := range failed { - fmt.Fprintf(os.Stderr, "- [ ] %s (%d errors)\n", repo, len(repo.Errors)) - for _, err := range repo.Errors { - fmt.Fprintf(os.Stderr, " - [ ] %s\n", err) + n := 0 + // Only show warning-like errors in verbose output + if ! verbose { + for _, err := range repo.Errors { + switch err.(type) { + case *MissingIncludeError, *UnknownKeysError: + break + default: + repo.Errors[n] = err + n++ + } + } + repo.Errors = repo.Errors[:n] + } + if len(repo.Errors) > 0 { + fmt.Fprintf(os.Stderr, "- [ ] %s (%d errors)\n", repo, len(repo.Errors)) + for _, err := range repo.Errors { + fmt.Fprintf(os.Stderr, " - %s\n", err) + } + fmt.Fprintf(os.Stderr, "\n") } - fmt.Fprintf(os.Stderr, "\n") total += len(repo.Errors) } if total > 0 { - return fmt.Errorf("the grammar library contains %d errors", total) + return fmt.Errorf("The grammar library contains %d errors", total) } return nil } diff --git a/tools/grammars/go.mod b/tools/grammars/go.mod new file mode 100644 index 0000000000..5aaf94ccc9 --- /dev/null +++ b/tools/grammars/go.mod @@ -0,0 +1,16 @@ +module github.com/github/linguist/tools/grammars + +go 1.16 + +require ( + github.com/fatih/color v1.12.0 // indirect + github.com/golang/protobuf v0.0.0-20171113180720-1e59b77b52bf + github.com/groob/plist v0.0.0-20171013152701-7b367e0aa692 + github.com/mattn/go-runewidth v0.0.2 // indirect + github.com/mitchellh/mapstructure v0.0.0-20171017171808-06020f85339e + github.com/urfave/cli/v2 v2.3.0 + golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect + gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect + gopkg.in/cheggaaa/pb.v1 v1.0.18 + gopkg.in/yaml.v2 v2.2.3 +) diff --git a/tools/grammars/go.sum b/tools/grammars/go.sum new file mode 100644 index 0000000000..6c7fb1de4d --- /dev/null +++ b/tools/grammars/go.sum @@ -0,0 +1,42 @@ +github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d h1:U+s90UTSYgptZMwQh2aRr3LuazLJIa+Pg3Kc1ylSYVY= +github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= +github.com/fatih/color v1.12.0 h1:mRhaKNwANqRgUBGKmnI5ZxEk7QXmjQeCcuYFMX2bfcc= +github.com/fatih/color v1.12.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGEBuJM= +github.com/golang/protobuf v0.0.0-20171113180720-1e59b77b52bf h1:pFr/u+m8QUBMW/itAczltF3guNRAL7XDs5tD3f6nSD0= +github.com/golang/protobuf v0.0.0-20171113180720-1e59b77b52bf/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/groob/plist v0.0.0-20171013152701-7b367e0aa692 h1:F3uWF39OwKCeQswABIG9NCYJ1+yAzA9+Smd3QphihjA= +github.com/groob/plist v0.0.0-20171013152701-7b367e0aa692/go.mod h1:qg2Nek0ND/hIr+nY8H1oVqEW2cLzVVNaAQ0QexOyjyc= +github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= +github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/mattn/go-colorable v0.1.8 h1:c1ghPdyEDarC70ftn0y+A/Ee++9zz8ljHG1b13eJ0s8= +github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= +github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY= +github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= +github.com/mattn/go-runewidth v0.0.2 h1:UnlwIPBGaTZfPQ6T1IGzPI0EkYAQmT9fAEJ/poFC63o= +github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU= +github.com/mitchellh/mapstructure v0.0.0-20171017171808-06020f85339e h1:PtGHLB3CX3TFPcksODQMxncoeQKWwCgTg0bJ40VLJP4= +github.com/mitchellh/mapstructure v0.0.0-20171017171808-06020f85339e/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q= +github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/shurcooL/sanitized_anchor_name v1.0.0 h1:PdmoCO6wvbs+7yrJyMORt4/BmY5IYyJwS/kOiWx8mHo= +github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= +github.com/urfave/cli/v2 v2.3.0 h1:qph92Y649prgesehzOrQjdWyxFOp/QVM+6imKHad91M= +github.com/urfave/cli/v2 v2.3.0/go.mod h1:LJmUH05zAU44vOAcrfzZQKsZbVcdbOG8rtL3/XcUArI= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c h1:5KslGYwFpkhGh+Q16bwMP3cOontH8FOep7tGV86Y7SQ= +golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae h1:/WDfKMnPU+m5M4xB+6x4kaepxRw6jWvR5iDRdvjHgy8= +golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= +gopkg.in/cheggaaa/pb.v1 v1.0.18 h1:h5Qflf8N54NDtm3lWfBuCD4rslDjkXDoGkEMZCH4R80= +gopkg.in/cheggaaa/pb.v1 v1.0.18/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qStrOgw= +gopkg.in/yaml.v2 v2.2.3 h1:fvjTMHxHEw/mxHbtzPi3JCcKXQRAnQTBRo6YCJSVHKI= +gopkg.in/yaml.v2 v2.2.3/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=