Treat grammar warnings as non-fatal (github-linguist#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

@@ -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')

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"]

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

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
 }

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
+)