Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify jsonnet-lint to accept multiple input files #545

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 21 additions & 25 deletions cmd/jsonnet-lint/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func version(o io.Writer) {
func usage(o io.Writer) {
version(o)
fmt.Fprintln(o)
fmt.Fprintln(o, "jsonnet-lint {<option>} { <filename> }")
fmt.Fprintln(o, "jsonnet-lint {<option>} { <filenames ...> }")
fmt.Fprintln(o)
fmt.Fprintln(o, "Available options:")
fmt.Fprintln(o, " -h / --help This message")
Expand Down Expand Up @@ -53,8 +53,8 @@ func usage(o io.Writer) {

type config struct {
// TODO(sbarzowski) Allow multiple root files checked at once for greater efficiency
inputFile string
evalJpath []string
inputFiles []string
evalJpath []string
}

func makeConfig() config {
Expand Down Expand Up @@ -112,11 +112,7 @@ func processArgs(givenArgs []string, config *config, vm *jsonnet.VM) (processArg
return processArgsStatusFailureUsage, fmt.Errorf("file not provided")
}

if len(remainingArgs) > 1 {
return processArgsStatusFailure, fmt.Errorf("only one file is allowed")
}

config.inputFile = remainingArgs[0]
config.inputFiles = remainingArgs
return processArgsStatusContinue, nil
}

Expand Down Expand Up @@ -164,27 +160,27 @@ func main() {
JPaths: config.evalJpath,
})

inputFile, err := os.Open(config.inputFile)
if err != nil {
die(err)
}
data, err := ioutil.ReadAll(inputFile)
if err != nil {
die(err)
}
err = inputFile.Close()
if err != nil {
die(err)
var snippets []linter.Snippet
for _, inputFile := range config.inputFiles {
f, err := os.Open(inputFile)
if err != nil {
die(err)
}
data, err := ioutil.ReadAll(f)
if err != nil {
die(err)
}
err = f.Close()
if err != nil {
die(err)
}

snippets = append(snippets, linter.Snippet{FileName: inputFile, Code: string(data)})
}

cmd.MemProfile()

_, err = jsonnet.SnippetToAST(config.inputFile, string(data))
if err != nil {
die(err)
}

errorsFound := linter.LintSnippet(vm, os.Stderr, config.inputFile, string(data))
errorsFound := linter.LintSnippet(vm, os.Stderr, snippets)
if errorsFound {
fmt.Fprintf(os.Stderr, "Problems found!\n")
os.Exit(2)
Expand Down
73 changes: 46 additions & 27 deletions linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ type ErrorWriter struct {
Writer io.Writer
}

// Snippet represents a jsonnet file data that to be linted
type Snippet struct {
FileName string
Code string
}

func (e *ErrorWriter) writeError(vm *jsonnet.VM, err errors.StaticError) {
e.ErrorsFound = true
_, writeErr := e.Writer.Write([]byte(vm.ErrorFormatter.Format(err) + "\n"))
Expand All @@ -38,10 +44,14 @@ type nodeWithLocation struct {
}

// Lint analyses a node and reports any issues it encounters to an error writer.
func lint(vm *jsonnet.VM, node nodeWithLocation, errWriter *ErrorWriter) {
func lint(vm *jsonnet.VM, nodes []nodeWithLocation, errWriter *ErrorWriter) {
roots := make(map[string]ast.Node)
roots[node.path] = node.node
getImports(vm, node, roots, errWriter)
for _, node := range nodes {
roots[node.path] = node.node
}
for _, node := range nodes {
getImports(vm, node, roots, errWriter)
}

variablesInFile := make(map[string]common.VariableInfo)

Expand All @@ -55,35 +65,38 @@ func lint(vm *jsonnet.VM, node nodeWithLocation, errWriter *ErrorWriter) {
return variables.FindVariables(node.node, variables.Environment{"std": &std})
}

variableInfo := findVariables(node)
for importedPath, rootNode := range roots {
variablesInFile[importedPath] = *findVariables(nodeWithLocation{rootNode, importedPath})
}

for _, v := range variableInfo.Variables {
if len(v.Occurences) == 0 && v.VariableKind == common.VarRegular && v.Name != "$" {
errWriter.writeError(vm, errors.MakeStaticError("Unused variable: "+string(v.Name), v.LocRange))
}
}
ec := common.ErrCollector{}

vars := make(map[string]map[ast.Node]*common.Variable)
for importedPath, info := range variablesInFile {
vars[importedPath] = info.VarAt
}

types.Check(node.node, roots, vars, func(currentPath, importedPath string) ast.Node {
node, _, err := vm.ImportAST(currentPath, importedPath)
if err != nil {
return nil
for _, node := range nodes {
variableInfo := findVariables(node)

for _, v := range variableInfo.Variables {
if len(v.Occurences) == 0 && v.VariableKind == common.VarRegular && v.Name != "$" {
errWriter.writeError(vm, errors.MakeStaticError("Unused variable: "+string(v.Name), v.LocRange))
}
}
return node
}, &ec)
ec := common.ErrCollector{}

types.Check(node.node, roots, vars, func(currentPath, importedPath string) ast.Node {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really take advantage of processing multiple files at once (it still builds the whole graph of type relationships each time). This is fine, though. We can do it as a separate change (which will be a performance-only change with no significant effect on the results).

node, _, err := vm.ImportAST(currentPath, importedPath)
if err != nil {
return nil
}
return node
}, &ec)

traversal.Traverse(node.node, &ec)
traversal.Traverse(node.node, &ec)

for _, err := range ec.Errs {
errWriter.writeError(vm, err)
for _, err := range ec.Errs {
errWriter.writeError(vm, err)
}
}
}

Expand Down Expand Up @@ -118,18 +131,24 @@ func getImports(vm *jsonnet.VM, node nodeWithLocation, roots map[string]ast.Node
}
}

// LintSnippet checks for problems in a single code snippet.
func LintSnippet(vm *jsonnet.VM, output io.Writer, filename, code string) bool {
// LintSnippet checks for problems in code snippet(s).
func LintSnippet(vm *jsonnet.VM, output io.Writer, snippets []Snippet) bool {
errWriter := ErrorWriter{
Writer: output,
ErrorsFound: false,
}

node, err := jsonnet.SnippetToAST(filename, code)
if err != nil {
errWriter.writeError(vm, err.(errors.StaticError)) // ugly but true
return true
var nodes []nodeWithLocation
for _, snippet := range snippets {
node, err := jsonnet.SnippetToAST(snippet.FileName, snippet.Code)

if err != nil {
errWriter.writeError(vm, err.(errors.StaticError)) // ugly but true
} else {
nodes = append(nodes, nodeWithLocation{node, snippet.FileName})
}
}
lint(vm, nodeWithLocation{node, filename}, &errWriter)

lint(vm, nodes, &errWriter)
return errWriter.ErrorsFound
}
40 changes: 39 additions & 1 deletion linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func runTest(t *testing.T, test *linterTest, changedGoldensList *ChangedGoldensL

var outBuilder strings.Builder

errorsFound := LintSnippet(vm, &outBuilder, test.name, string(input))
errorsFound := LintSnippet(vm, &outBuilder, []Snippet{Snippet{FileName: test.name, Code: string(input)}})

outData := outBuilder.String()

Expand Down Expand Up @@ -72,6 +72,40 @@ func runTest(t *testing.T, test *linterTest, changedGoldensList *ChangedGoldensL
}
}

func runTests(t *testing.T, tests []*linterTest) {
read := func(file string) []byte {
bytz, err := ioutil.ReadFile(file)
if err != nil {
t.Fatalf("reading file: %s: %v", file, err)
}
return bytz
}

vm := jsonnet.MakeVM()

var snippets []Snippet

for _, test := range tests {
input := read(test.input)

snippets = append(snippets, Snippet{FileName: test.name, Code: string(input)})
}

var outBuilder strings.Builder

errorsFound := LintSnippet(vm, &outBuilder, snippets)

outData := outBuilder.String()

if outData == "" && errorsFound {
t.Error(fmt.Errorf("return value indicates problems present, but no output was produced"))
}

if outData != "" && !errorsFound {
t.Error(fmt.Errorf("return value indicates no problems, but output is not empty:\n%v", outData))
}
}

func TestLinter(t *testing.T) {
flag.Parse()

Expand Down Expand Up @@ -122,4 +156,8 @@ func TestLinter(t *testing.T) {
t.Fail()
})
}

t.Run("passing multiple input files", func(t *testing.T) {
runTests(t, tests)
})
}