compiler: Add error output to the compiler (#3935)

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
+  opts.on("-q", "--quiet", "Do not print output unless there's a failure") do
-    $verbose = true
+    $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}`
+  command('git', 'submodule', 'deinit', repo_old)
-  `git rm -rf #{repo_old}`
+  command('git', 'rm', '-rf', repo_old)
-  `script/grammar-compiler -update`
+  command('script/grammar-compiler', 'update', '-f')
 end
 
 log "Registering new submodule: #{repo_new}"
-`git submodule add -f #{https} #{repo_new}`
+command('git', 'submodule', 'add', '-f', https, repo_new)
-exit 1 if $?.exitstatus > 0
+command('script/grammar-compiler', 'add', repo_new)
-`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`
+command('bundle', 'exec', 'rake', 'samples')
-`script/list-grammars`
+command('script/list-grammars')

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 "$@"

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

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"

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")
+func cwd() string {
-var protoOut = flag.String("proto", "", "dump Protobuf library")
+	cwd, _ := os.Getwd()
-var jsonOut = flag.String("json", "", "dump JSON output")
+	return cwd
-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 fatal(err error) {
+func wrap(err error) error {
-	fmt.Fprintf(os.Stderr, "FATAL: %s\n", err)
+	return cli.NewExitError(err, 255)
-	os.Exit(1)
 }
 
 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 {
+	app.Flags = []cli.Flag{
-		fatal(err)
+		cli.StringFlag{
+			Name:  "linguist-path",
+			Value: cwd(),
+			Usage: "path to Linguist root",
+		},
 	}
 
-	if *linguistRoot == "" {
+	app.Commands = []cli.Command{
-		cwd, err := os.Getwd()
+		{
-		if err != nil {
+			Name:  "add",
-			fatal(err)
+			Usage: "add a new grammar source",
-		}
+			Flags: []cli.Flag{
-		*linguistRoot = cwd
+				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)
+	app.Run(os.Args)
-	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()
-	}
 }

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

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,

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
 }