From 2b39e12453547333708712c2428df8b78828951e Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Sun, 24 Feb 2019 14:51:33 +0000 Subject: [PATCH] Update linter to use go vet (#563) * couple of smol tweaks * Updating linter to use go vet * Strip these struct tags, we don't need them any more * Fix a bunch of legit vet warnings * Fix final linter issues * Add gofmt too * gofmt everything * buildifier * golint * golint * apparently alias doesn't work like that --- bootstrap.sh | 8 - src/cache/rpc_cache.go | 2 +- src/cache/rpc_cache_stub.go | 2 +- src/core/config.go | 2 +- src/ide/intellij/library.go | 14 +- src/ide/intellij/library_test.go | 2 +- src/ide/intellij/module.go | 18 +- src/ide/intellij/project.go | 16 +- src/output/BUILD | 1 + src/output/shell_output.go | 4 - src/output/trace.go | 14 +- src/parse/asp/grammar.go | 182 +++++++++--------- src/parse/asp/parser.go | 9 +- src/parse/rules/go_rules.build_defs | 2 +- src/please.go | 2 +- src/query/reverse_deps.go | 2 +- src/query/roots.go | 2 +- src/query/whatoutputs.go | 2 +- src/test/BUILD | 1 - src/test/surefire.go | 4 +- src/test/xml_coverage.go | 8 +- src/test/xml_results.go | 10 +- tools/build_langserver/langserver/analyzer.go | 2 +- .../build_langserver/langserver/completion.go | 2 +- .../langserver/diagnostics.go | 2 +- .../langserver/diagnostics_test.go | 2 +- .../langserver/handler_test.go | 4 +- tools/build_langserver/langserver/rename.go | 2 +- .../build_langserver/langserver/setup_test.go | 2 +- tools/build_langserver/langserver/utils.go | 2 +- .../build_langserver/langserver/utils_test.go | 2 +- tools/build_langserver/langserver_main.go | 5 +- tools/build_langserver/lsp/specs.go | 4 +- tools/build_langserver/lsp/text_document.go | 4 +- tools/jarcat/unzip/unzip.go | 2 +- tools/jarcat/zip/writer.go | 2 +- tools/misc/BUILD | 4 +- tools/misc/ci_lint.py | 83 -------- tools/misc/lint.sh | 30 +++ 39 files changed, 202 insertions(+), 259 deletions(-) delete mode 100755 tools/misc/ci_lint.py create mode 100644 tools/misc/lint.sh diff --git a/bootstrap.sh b/bootstrap.sh index 8b318eccfa..6ec05c7cb0 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -89,11 +89,3 @@ plz-out/bin/src/please $PLZ_ARGS ${PLZ_COVER:-test} $EXCLUDES --exclude=e2e --lo # finicky about some things due to running plz recursively and disabling the lock. notice "Running end-to-end tests..." plz-out/bin/src/please $PLZ_ARGS ${PLZ_COVER:-test} $EXCLUDES --include=e2e --log_file plz-out/log/test_build.log --log_file_level 4 $@ - -# Lint needs python3. -if hash python3 2>/dev/null ; then - # Don't run this in CI or any unusual workflows. - if [ $# -eq 0 ] ; then - plz lint - fi -fi diff --git a/src/cache/rpc_cache.go b/src/cache/rpc_cache.go index 96ae0ded31..403339e606 100644 --- a/src/cache/rpc_cache.go +++ b/src/cache/rpc_cache.go @@ -6,11 +6,11 @@ package cache import ( "bytes" - "github.com/thought-machine/please/src/core" "crypto/tls" "crypto/x509" "encoding/base64" "fmt" + "github.com/thought-machine/please/src/core" "io/ioutil" "os" "path" diff --git a/src/cache/rpc_cache_stub.go b/src/cache/rpc_cache_stub.go index 3251205756..155cb5ed37 100644 --- a/src/cache/rpc_cache_stub.go +++ b/src/cache/rpc_cache_stub.go @@ -6,8 +6,8 @@ package cache import ( - "github.com/thought-machine/please/src/core" "fmt" + "github.com/thought-machine/please/src/core" ) func newRPCCache(config *core.Configuration) (*httpCache, error) { diff --git a/src/core/config.go b/src/core/config.go index 0a95ed1440..3548c02de8 100644 --- a/src/core/config.go +++ b/src/core/config.go @@ -429,7 +429,7 @@ type Configuration struct { DefaultNamespace string `help:"Namespace passed to all cc_embed_binary rules when not overridden by the namespace argument to that rule.\nNot set by default, if you want to use those rules you'll need to set it or pass it explicitly to each one." var:"DEFAULT_NAMESPACE"` PkgConfigPath string `help:"Custom PKG_CONFIG_PATH for pkg-config.\nBy default this is empty." var:"PKG_CONFIG_PATH"` Coverage bool `help:"If true (the default), coverage will be available for C and C++ build rules.\nThis is still a little experimental but should work for GCC. Right now it does not work for Clang (it likely will in Clang 4.0 which will likely support --fprofile-dir) and so this can be useful to disable it.\nIt's also useful in some cases for CI systems etc if you'd prefer to avoid the overhead, since the tests have to be compiled with extra instrumentation and without optimisation." var:"CPP_COVERAGE"` - TestMain BuildLabel `help:"The build target to use for the default main for C++ test rules." example:"@pleasings//cc:unittest_main" var:"CC_TEST_MAIN"` + TestMain BuildLabel `help:"The build target to use for the default main for C++ test rules." example:"///pleasings//cc:unittest_main" var:"CC_TEST_MAIN"` ClangModules bool `help:"Uses Clang-style arguments for compiling cc_module rules. If disabled gcc-style arguments will be used instead. Experimental, expected to be removed at some point once module compilation methods are more consistent." var:"CC_MODULES_CLANG"` } `help:"Please has built-in support for compiling C and C++ code. We don't support every possible nuance of compilation for these languages, but aim to provide something fairly straightforward.\nTypically there is little problem compiling & linking against system libraries although Please has no insight into those libraries and when they change, so cannot rebuild targets appropriately.\n\nThe C and C++ rules are very similar and simply take a different set of tools and flags to facilitate side-by-side usage."` Proto struct { diff --git a/src/ide/intellij/library.go b/src/ide/intellij/library.go index 04473e62b0..69eb8b0d5c 100644 --- a/src/ide/intellij/library.go +++ b/src/ide/intellij/library.go @@ -9,8 +9,8 @@ import ( type libraryComponent struct { XMLName xml.Name `xml:"component"` - Name string `xml:"name,attr"` - Library Library `xml:"library"` + Name string `xml:"name,attr"` + Library Library `xml:"library"` } // Content is a simple wrapper for a URL. @@ -33,7 +33,7 @@ func newLibrary(graph *core.BuildGraph, target *core.BuildTarget) Library { classes := []Content{} javadocs := []Content{} sources := []Content{} - for _, dep := range target.Sources { + for _, dep := range target.Sources { label := dep.Label() if label != nil { depTarget := graph.TargetOrDie(*label) @@ -65,9 +65,9 @@ func newLibrary(graph *core.BuildGraph, target *core.BuildTarget) Library { } library := Library{ - Name: libraryName(target), - SourcePaths: sources, - ClassPaths: classes, + Name: libraryName(target), + SourcePaths: sources, + ClassPaths: classes, JavadocPaths: javadocs, } @@ -76,7 +76,7 @@ func newLibrary(graph *core.BuildGraph, target *core.BuildTarget) Library { func (library *Library) toXML(writer io.Writer) { table := libraryComponent{ - Name: "libraryTable", + Name: "libraryTable", Library: *library, } contents, err := xml.MarshalIndent(table, "", " ") diff --git a/src/ide/intellij/library_test.go b/src/ide/intellij/library_test.go index 47d4b1098d..ff2fbc407f 100644 --- a/src/ide/intellij/library_test.go +++ b/src/ide/intellij/library_test.go @@ -25,7 +25,7 @@ func TestLibraryToXml(t *testing.T) { buf := &bytes.Buffer{} library.toXML(buf) assert.Equal(t, - "\n"+ + "\n"+ " \n"+ " \n"+ " \n"+ diff --git a/src/ide/intellij/module.go b/src/ide/intellij/module.go index d0f7f4fb07..d1dd7863b0 100644 --- a/src/ide/intellij/module.go +++ b/src/ide/intellij/module.go @@ -11,10 +11,10 @@ import ( // Module represents the IntelliJ concept of a module type Module struct { - XMLName xml.Name `xml:"module"` - ModuleType string `xml:"type,attr"` - Version int `xml:"version,attr"` - Component ModuleComponent `xml:"component"` + XMLName xml.Name `xml:"module"` + ModuleType string `xml:"type,attr"` + Version int `xml:"version,attr"` + Component ModuleComponent `xml:"component"` } func newModule(graph *core.BuildGraph, target *core.BuildTarget) Module { @@ -31,7 +31,7 @@ func newModule(graph *core.BuildGraph, target *core.BuildTarget) Module { module := Module{ ModuleType: "WEB_MODULE", Version: 4, - Component: component, + Component: component, } return module @@ -135,7 +135,7 @@ func (module *Module) toXML(writer io.Writer) { writer.Write([]byte("\n")) content, err := xml.MarshalIndent(module, "", " ") - if err == nil { + if err == nil { writer.Write(content) } } @@ -153,7 +153,7 @@ func newModuleComponent(target *core.BuildTarget) ModuleComponent { orderEntries := []OrderEntry{} return ModuleComponent{ - Name: "NewModuleRootManager", + Name: "NewModuleRootManager", InheritCompilerOutput: true, Content: newModuleContent(target), OrderEntries: orderEntries, @@ -231,7 +231,7 @@ func newLibraryEntry(name, level string) OrderEntry { Type: "library", LibraryName: &name, LibraryLevel: &level, - Exported: &exported, + Exported: &exported, } } @@ -240,7 +240,7 @@ func newModuleEntry(name string) OrderEntry { return OrderEntry{ Type: "module", ModuleName: &name, - Exported: &exported, + Exported: &exported, } } diff --git a/src/ide/intellij/project.go b/src/ide/intellij/project.go index 71c5e4558e..100ff56f5e 100644 --- a/src/ide/intellij/project.go +++ b/src/ide/intellij/project.go @@ -1,9 +1,9 @@ package intellij import ( - "github.com/thought-machine/please/src/core" "encoding/xml" "fmt" + "github.com/thought-machine/please/src/core" "io" ) @@ -25,7 +25,7 @@ func (misc *Misc) toXML(w io.Writer) { w.Write([]byte("\n")) content, err := xml.MarshalIndent(misc, "", " ") - if err == nil { + if err == nil { w.Write(content) } } @@ -72,8 +72,8 @@ func newMiscOutput() MiscOutput { // Modules are the main structure that tells IntelliJ where to find all the modules it knows about. type Modules struct { - XMLName xml.Name `xml:"project"` - Version int `xml:"version,attr"` + XMLName xml.Name `xml:"project"` + Version int `xml:"version,attr"` Component ModulesComponent `xml:"component"` } @@ -88,15 +88,15 @@ func (modules *Modules) toXML(w io.Writer) { w.Write([]byte("\n")) content, err := xml.MarshalIndent(modules, "", " ") - if err == nil { + if err == nil { w.Write(content) } } // ModulesComponent represents all modules in the workspace. type ModulesComponent struct { - XMLName xml.Name `xml:"component"` - Name string `xml:"name,attr"` + XMLName xml.Name `xml:"component"` + Name string `xml:"name,attr"` Modules []ModulesModule `xml:"modules>module"` } @@ -122,7 +122,7 @@ type ModulesModule struct { func newModulesModule(label core.BuildLabel) ModulesModule { filePath := "$PROJECT_DIR$/" + label.PackageDir() + "/" + fmt.Sprintf("%s.iml", moduleName(label)) return ModulesModule{ - FileURL: "file://"+filePath, + FileURL: "file://" + filePath, FilePath: filePath, } } diff --git a/src/output/BUILD b/src/output/BUILD index 9012687316..4ce1375b8e 100644 --- a/src/output/BUILD +++ b/src/output/BUILD @@ -12,6 +12,7 @@ go_library( name = "output", srcs = [ "print.go", + "print_bootstrap.go", "trace.go", ":ansi_replacements", ], diff --git a/src/output/shell_output.go b/src/output/shell_output.go index 9a54534bf4..75a5a275fc 100644 --- a/src/output/shell_output.go +++ b/src/output/shell_output.go @@ -16,16 +16,12 @@ import ( "sync" "time" - "gopkg.in/op/go-logging.v1" - "github.com/thought-machine/please/src/build" "github.com/thought-machine/please/src/cli" "github.com/thought-machine/please/src/core" "github.com/thought-machine/please/src/test" ) -var log = logging.MustGetLogger("output") - // durationGranularity is the granularity that we build durations at. const durationGranularity = 10 * time.Millisecond const testDurationGranularity = time.Millisecond diff --git a/src/output/trace.go b/src/output/trace.go index 55505714cf..335930a25a 100644 --- a/src/output/trace.go +++ b/src/output/trace.go @@ -3,11 +3,17 @@ package output -import "encoding/json" -import "fmt" -import "os" +import ( + "encoding/json" + "fmt" + "os" -import "github.com/thought-machine/please/src/core" + "gopkg.in/op/go-logging.v1" + + "github.com/thought-machine/please/src/core" +) + +var log = logging.MustGetLogger("output") var traces = make([]traceEntry, 0, 1000) diff --git a/src/parse/asp/grammar.go b/src/parse/asp/grammar.go index e2cee07375..e5758fa3a3 100644 --- a/src/parse/asp/grammar.go +++ b/src/parse/asp/grammar.go @@ -4,7 +4,7 @@ import "fmt" // A FileInput is the top-level structure of a BUILD file. type FileInput struct { - Statements []*Statement `{ @@ } EOF` + Statements []*Statement } // A Position describes a position in a source file. @@ -28,32 +28,32 @@ func (pos Position) String() string { type Statement struct { Pos Position EndPos Position - FuncDef *FuncDef `| @@` - For *ForStatement `| @@` - If *IfStatement `| @@` - Return *ReturnStatement `| "return" @@ EOL` - Raise *Expression `| "raise" @@ EOL` + FuncDef *FuncDef + For *ForStatement + If *IfStatement + Return *ReturnStatement + Raise *Expression Assert *struct { - Expr *Expression `@@` - Message string `["," @String]` - } `| "assert" @@ EOL` - Ident *IdentStatement `| @@ EOL` - Literal *Expression `| @@ EOL)` - Pass bool `( @"pass" EOL` - Continue bool `| @"continue" EOL` + Expr *Expression + Message string + } + Ident *IdentStatement + Literal *Expression + Pass bool + Continue bool } // A ReturnStatement implements the Python 'return' statement. type ReturnStatement struct { - Values []*Expression `[ @@ { "," @@ } ]` + Values []*Expression } // A FuncDef implements definition of a new function. type FuncDef struct { - Name string `"def" @Ident` - Arguments []Argument `"(" [ @@ { "," @@ } ] ")" Colon EOL` - Docstring string `[ @String EOL ]` - Statements []*Statement `{ @@ } Unindent` + Name string + Arguments []Argument + Docstring string + Statements []*Statement EoDef Position // allowed return type of the FuncDef Return string @@ -66,30 +66,30 @@ type FuncDef struct { // A ForStatement implements the 'for' statement. // Note that it does not support Python's "for-else" construction. type ForStatement struct { - Names []string `"for" @Ident [ { "," @Ident } ] "in"` - Expr Expression `@@ Colon EOL` - Statements []*Statement `{ @@ } Unindent` + Names []string + Expr Expression + Statements []*Statement } // An IfStatement implements the if-elif-else statement. type IfStatement struct { - Condition Expression `"if" @@ Colon EOL` - Statements []*Statement `{ @@ } Unindent` + Condition Expression + Statements []*Statement Elif []struct { - Condition Expression `"elif" @@ Colon EOL` - Statements []*Statement `{ @@ } Unindent` - } `{ @@ }` - ElseStatements []*Statement `[ "else" Colon EOL { @@ } Unindent ]` + Condition Expression + Statements []*Statement + } + ElseStatements []*Statement } // An Argument represents an argument to a function definition. type Argument struct { - Name string `@Ident` - Type []string `[ ":" @( { ( "bool" | "str" | "int" | "list" | "dict" | "function" ) [ "|" ] } ) ]` + Name string + Type []string // Aliases are an experimental non-Python concept where function arguments can be aliased to different names. // We use this to support compatibility with Bazel & Buck etc in some cases. - Aliases []string `[ "&" ( { @Ident [ "&" ] } ) ]` - Value *Expression `[ "=" @@ ]` + Aliases []string + Value *Expression IsPrivate bool } @@ -99,10 +99,10 @@ type Argument struct { type Expression struct { Pos Position EndPos Position - UnaryOp *UnaryOp `( @@` - Val *ValueExpression `| @@ )` - Op []OpExpression `{ @@ }` - If *InlineIf `[ @@ ]` + UnaryOp *UnaryOp + Val *ValueExpression + Op []OpExpression + If *InlineIf // For internal optimisation - do not use outside this package. Optimised *OptimisedExpression } @@ -120,26 +120,26 @@ type OptimisedExpression struct { // An OpExpression is a operator combined with its following expression. type OpExpression struct { - Op Operator `@("+" | "-" | "%" | "<" | ">" | "and" | "or" | "is" | "in" | "not" "in" | "==" | "!=" | ">=" | "<=")` - Expr *Expression `@@` + Op Operator + Expr *Expression } // A ValueExpression is the value part of an expression, i.e. without surrounding operators. type ValueExpression struct { - String string `( @String` - FString *FString `| @FString` + String string + FString *FString Int *struct { - Int int `@Int` - } `| @@` // Should just be *int, but https://github.com/golang/go/issues/23498 :( - Bool string `| @( "True" | "False" | "None" )` - List *List `| "[" @@ "]"` - Dict *Dict `| "{" @@ "}"` - Tuple *List `| "(" @@ ")"` - Lambda *Lambda `| "lambda" @@` - Ident *IdentExpr `| @@ )` - Slice *Slice `[ @@ ]` - Property *IdentExpr `[ ( "." @@` - Call *Call `| "(" @@ ")" ) ]` + Int int + } // Should just be *int, but https://github.com/golang/go/issues/23498 :( + Bool string + List *List + Dict *Dict + Tuple *List + Lambda *Lambda + Ident *IdentExpr + Slice *Slice + Property *IdentExpr + Call *Call } // A FString represents a minimal version of a Python literal format string. @@ -156,32 +156,32 @@ type FString struct { // A UnaryOp represents a unary operation - in our case the only ones we support are negation and not. type UnaryOp struct { - Op string `@( "-" | "not" )` - Expr ValueExpression `@@` + Op string + Expr ValueExpression } // An IdentStatement implements a statement that begins with an identifier (i.e. anything that // starts off with a variable name). It is a little fiddly due to parser limitations. type IdentStatement struct { - Name string `@Ident` + Name string Unpack *struct { - Names []string `@Ident { "," @Ident }` - Expr *Expression `"=" @@` - } `( "," @@ ` + Names []string + Expr *Expression + } Index *struct { - Expr *Expression `@@ "]"` - Assign *Expression `( "=" @@` - AugAssign *Expression `| "+=" @@ )` - } `| "[" @@` - Action *IdentStatementAction `| @@ )` + Expr *Expression + Assign *Expression + AugAssign *Expression + } + Action *IdentStatementAction } // An IdentStatementAction implements actions on an IdentStatement. type IdentStatementAction struct { - Property *IdentExpr ` "." @@` - Call *Call `| "(" @@ ")"` - Assign *Expression `| "=" @@` - AugAssign *Expression `| "+=" @@` + Property *IdentExpr + Call *Call + Assign *Expression + AugAssign *Expression } // An IdentExpr implements parts of an expression that begin with an identifier (i.e. anything @@ -189,71 +189,71 @@ type IdentStatementAction struct { type IdentExpr struct { Pos Position EndPos Position - Name string `@Ident` + Name string Action []struct { - Property *IdentExpr ` "." @@` - Call *Call `| "(" @@ ")"` - } `{ @@ }` + Property *IdentExpr + Call *Call + } } // A Call represents a call site of a function. type Call struct { - Arguments []CallArgument `[ @@ ] { "," [ @@ ] }` + Arguments []CallArgument } // A CallArgument represents a single argument at a call site of a function. type CallArgument struct { Pos Position - Name string `[ @@ "=" ]` - Value Expression `@@` + Name string + Value Expression } // A List represents a list literal, either with or without a comprehension clause. type List struct { - Values []*Expression `[ @@ ] { "," [ @@ ] }` - Comprehension *Comprehension `[ @@ ]` + Values []*Expression + Comprehension *Comprehension } // A Dict represents a dict literal, either with or without a comprehension clause. type Dict struct { - Items []*DictItem `[ @@ ] { "," [ @@ ] }` - Comprehension *Comprehension `[ @@ ]` + Items []*DictItem + Comprehension *Comprehension } // A DictItem represents a single key-value pair in a dict literal. type DictItem struct { - Key Expression `@( Ident | String ) ":"` - Value Expression `@@` + Key Expression + Value Expression } // A Slice represents a slice or index expression (e.g. [1], [1:2], [2:], [:], etc). type Slice struct { - Start *Expression `"[" [ @@ ]` - Colon string `[ @":" ]` - End *Expression `[ @@ ] "]"` + Start *Expression + Colon string + End *Expression } // An InlineIf implements the single-line if-then-else construction type InlineIf struct { - Condition *Expression `"if" @@` - Else *Expression `[ "else" @@ ]` + Condition *Expression + Else *Expression } // A Comprehension represents a list or dict comprehension clause. type Comprehension struct { - Names []string `"for" @Ident [ { "," @Ident } ] "in"` - Expr *Expression `@@` + Names []string + Expr *Expression Second *struct { - Names []string `"for" @Ident [ { "," @Ident } ] "in"` - Expr *Expression `@@` - } `[ @@ ]` - If *Expression `[ "if" @@ ]` + Names []string + Expr *Expression + } + If *Expression } // A Lambda is the inline lambda function. type Lambda struct { - Arguments []Argument `[ @@ { "," @@ } ] Colon` - Expr Expression `@@` + Arguments []Argument + Expr Expression } // An Operator defines a unary or binary operator. diff --git a/src/parse/asp/parser.go b/src/parse/asp/parser.go index 85b1c99174..82f892913b 100644 --- a/src/parse/asp/parser.go +++ b/src/parse/asp/parser.go @@ -227,9 +227,10 @@ func whitelistedKwargs(name, filename string) bool { return true // Don't care about anything private, or non-rule builtins. } return map[string]bool{ - "workspace": true, - "decompose": true, - "check_config": true, - "select": true, + "workspace": true, + "decompose": true, + "check_config": true, + "select": true, + "exports_files": true, }[name] } diff --git a/src/parse/rules/go_rules.build_defs b/src/parse/rules/go_rules.build_defs index 15cb4375c7..83f26c5646 100644 --- a/src/parse/rules/go_rules.build_defs +++ b/src/parse/rules/go_rules.build_defs @@ -42,7 +42,7 @@ def go_library(name:str, srcs:list, asm_srcs:list=None, hdrs:list=None, out:str= src_labels = [] private = out.startswith('_') if _link_private or not private: - src_labels = ['link:plz-out/go/src/${PKG}'] + src_labels = ['link:plz-out/go/src/${PKG}', 'go_src'] if not private: labels = ['link:plz-out/go/pkg/${OS}_${ARCH}/${PKG}'] libname = out[:-2] if len(out) > 2 else out diff --git a/src/please.go b/src/please.go index 681348e19a..8d6235f874 100644 --- a/src/please.go +++ b/src/please.go @@ -706,7 +706,7 @@ func doTest(targets []core.BuildLabel, surefireDir cli.Filepath, resultsFile cli os.RemoveAll(string(resultsFile)) os.MkdirAll(string(surefireDir), core.DirPermissions) success, state := runBuild(targets, true, true) - test.CopySurefireXmlFilesToDir(state, string(surefireDir)) + test.CopySurefireXMLFilesToDir(state, string(surefireDir)) test.WriteResultsToFileOrDie(state.Graph, string(resultsFile)) return success, state } diff --git a/src/query/reverse_deps.go b/src/query/reverse_deps.go index 5c075eff82..b5c14058a3 100644 --- a/src/query/reverse_deps.go +++ b/src/query/reverse_deps.go @@ -1,8 +1,8 @@ package query import ( - "github.com/thought-machine/please/src/core" "fmt" + "github.com/thought-machine/please/src/core" "sort" ) diff --git a/src/query/roots.go b/src/query/roots.go index 13525821a6..5d23c5ab6b 100644 --- a/src/query/roots.go +++ b/src/query/roots.go @@ -1,8 +1,8 @@ package query import ( - "github.com/thought-machine/please/src/core" "fmt" + "github.com/thought-machine/please/src/core" "sort" ) diff --git a/src/query/whatoutputs.go b/src/query/whatoutputs.go index b6b4fefc8b..8edcdb5953 100644 --- a/src/query/whatoutputs.go +++ b/src/query/whatoutputs.go @@ -1,8 +1,8 @@ package query import ( - "github.com/thought-machine/please/src/core" "fmt" + "github.com/thought-machine/please/src/core" "path" ) diff --git a/src/test/BUILD b/src/test/BUILD index 9d51278ca9..f248eab48c 100644 --- a/src/test/BUILD +++ b/src/test/BUILD @@ -3,7 +3,6 @@ go_library( srcs = glob( ["*.go"], exclude = [ - "*_stub.go", "*_test.go", ], ), diff --git a/src/test/surefire.go b/src/test/surefire.go index bd58d0993a..7d4c654cbf 100644 --- a/src/test/surefire.go +++ b/src/test/surefire.go @@ -8,8 +8,8 @@ import ( "github.com/thought-machine/please/src/fs" ) -// CopySurefireXmlFilesToDir copies all the XML test results files into the given directory. -func CopySurefireXmlFilesToDir(state *core.BuildState, surefireDir string) { +// CopySurefireXMLFilesToDir copies all the XML test results files into the given directory. +func CopySurefireXMLFilesToDir(state *core.BuildState, surefireDir string) { for _, label := range state.ExpandOriginalLabels() { target := state.Graph.TargetOrDie(label) if state.ShouldInclude(target) && target.IsTest && !target.NoTestOutput { diff --git a/src/test/xml_coverage.go b/src/test/xml_coverage.go index a477465ac1..6a13df2963 100644 --- a/src/test/xml_coverage.go +++ b/src/test/xml_coverage.go @@ -3,9 +3,9 @@ package test import ( + "encoding/xml" "github.com/thought-machine/please/src/cli" "github.com/thought-machine/please/src/core" - "encoding/xml" "math" "path" "strings" @@ -118,12 +118,12 @@ func getLineCoverageInfo(lineCover []core.LineCoverage) ([]line, int, int) { if status == core.Covered { line := line{Hits: 1, Number: index} lines = append(lines, line) - covered += 1 - total += 1 + covered++ + total++ } else if status == core.Uncovered { line := line{Hits: 0, Number: index} lines = append(lines, line) - total += 1 + total++ } } diff --git a/src/test/xml_results.go b/src/test/xml_results.go index e59cd33171..5504d4ae60 100644 --- a/src/test/xml_results.go +++ b/src/test/xml_results.go @@ -421,11 +421,11 @@ func WriteResultsToFileOrDie(graph *core.BuildGraph, filename string) { Failures: testSuite.Failures(), Skipped: testSuite.Skips(), timed: timed{testSuite.Duration.Seconds()}, - Properties: toXmlProperties(testSuite.Properties), + Properties: toXMLProperties(testSuite.Properties), } } for _, testCase := range testSuite.TestCases { - xmlTest := toXmlTestCase(testCase) + xmlTest := toXMLTestCase(testCase) if xmlTest.ClassName == "" { xmlTest.ClassName = testSuite.JavaStyleName() } @@ -433,7 +433,7 @@ func WriteResultsToFileOrDie(graph *core.BuildGraph, filename string) { } xmlSuites[testSuite.JavaStyleName()] = xmlTestSuite for _, testCase := range testSuite.TestCases { - xmlTest := toXmlTestCase(testCase) + xmlTest := toXMLTestCase(testCase) xmlTestSuite.TestCases = append(xmlTestSuite.TestCases, xmlTest) } } @@ -450,7 +450,7 @@ func WriteResultsToFileOrDie(graph *core.BuildGraph, filename string) { } } -func toXmlProperties(props map[string]string) jUnitXMLProperties { +func toXMLProperties(props map[string]string) jUnitXMLProperties { out := jUnitXMLProperties{} for k, v := range props { out.Property = append(out.Property, jUnitXMLProperty{ @@ -461,7 +461,7 @@ func toXmlProperties(props map[string]string) jUnitXMLProperties { return out } -func toXmlTestCase(result core.TestCase) jUnitXMLTest { +func toXMLTestCase(result core.TestCase) jUnitXMLTest { testcase := jUnitXMLTest{ ClassName: result.ClassName, Name: result.Name, diff --git a/tools/build_langserver/langserver/analyzer.go b/tools/build_langserver/langserver/analyzer.go index 941f2455d9..3ae9f7bb8d 100644 --- a/tools/build_langserver/langserver/analyzer.go +++ b/tools/build_langserver/langserver/analyzer.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "path" "path/filepath" - "query" "regexp" "sort" "strconv" @@ -18,6 +17,7 @@ import ( "github.com/thought-machine/please/src/parse/asp" "github.com/thought-machine/please/src/parse/rules" "github.com/thought-machine/please/src/plz" + "github.com/thought-machine/please/src/query" "github.com/thought-machine/please/tools/build_langserver/lsp" ) diff --git a/tools/build_langserver/langserver/completion.go b/tools/build_langserver/langserver/completion.go index 8b859fbd78..d12ea21f32 100644 --- a/tools/build_langserver/langserver/completion.go +++ b/tools/build_langserver/langserver/completion.go @@ -2,9 +2,9 @@ package langserver import ( "context" - "github.com/thought-machine/please/src/core" "encoding/json" "fmt" + "github.com/thought-machine/please/src/core" "github.com/thought-machine/please/src/query" "strings" diff --git a/tools/build_langserver/langserver/diagnostics.go b/tools/build_langserver/langserver/diagnostics.go index a6b5f76cb1..d496d36d5f 100644 --- a/tools/build_langserver/langserver/diagnostics.go +++ b/tools/build_langserver/langserver/diagnostics.go @@ -2,8 +2,8 @@ package langserver import ( "context" - "github.com/thought-machine/please/src/core" "fmt" + "github.com/thought-machine/please/src/core" "github.com/thought-machine/please/src/parse/asp" "github.com/thought-machine/please/tools/build_langserver/lsp" diff --git a/tools/build_langserver/langserver/diagnostics_test.go b/tools/build_langserver/langserver/diagnostics_test.go index 572e65dae3..2319a650f2 100644 --- a/tools/build_langserver/langserver/diagnostics_test.go +++ b/tools/build_langserver/langserver/diagnostics_test.go @@ -2,8 +2,8 @@ package langserver import ( "context" - "testing" "github.com/thought-machine/please/tools/build_langserver/lsp" + "testing" "github.com/stretchr/testify/assert" ) diff --git a/tools/build_langserver/langserver/handler_test.go b/tools/build_langserver/langserver/handler_test.go index 478f2a97a5..5d0486f1ec 100644 --- a/tools/build_langserver/langserver/handler_test.go +++ b/tools/build_langserver/langserver/handler_test.go @@ -2,13 +2,13 @@ package langserver import ( "context" - "github.com/thought-machine/please/src/core" "github.com/sourcegraph/jsonrpc2" "github.com/stretchr/testify/assert" + "github.com/thought-machine/please/src/core" + "github.com/thought-machine/please/tools/build_langserver/lsp" "net" "strings" "testing" - "github.com/thought-machine/please/tools/build_langserver/lsp" ) // TODO(bnm): cleanup diff --git a/tools/build_langserver/langserver/rename.go b/tools/build_langserver/langserver/rename.go index 5e1087f5ef..7ae846690d 100644 --- a/tools/build_langserver/langserver/rename.go +++ b/tools/build_langserver/langserver/rename.go @@ -2,11 +2,11 @@ package langserver import ( "context" - "core" "encoding/json" "github.com/sourcegraph/jsonrpc2" + "github.com/thought-machine/please/src/core" "github.com/thought-machine/please/src/parse/asp" "github.com/thought-machine/please/tools/build_langserver/lsp" ) diff --git a/tools/build_langserver/langserver/setup_test.go b/tools/build_langserver/langserver/setup_test.go index 7dd0d7a187..014cc49c39 100644 --- a/tools/build_langserver/langserver/setup_test.go +++ b/tools/build_langserver/langserver/setup_test.go @@ -5,8 +5,8 @@ import ( "testing" "github.com/thought-machine/please/src/core" - "io/ioutil" "github.com/thought-machine/please/tools/build_langserver/lsp" + "io/ioutil" ) // TODO(bnm): cleanup setup diff --git a/tools/build_langserver/langserver/utils.go b/tools/build_langserver/langserver/utils.go index d2795a5ecd..ce9afdf10e 100644 --- a/tools/build_langserver/langserver/utils.go +++ b/tools/build_langserver/langserver/utils.go @@ -3,8 +3,8 @@ package langserver import ( "bufio" "context" - "github.com/thought-machine/please/src/core" "fmt" + "github.com/thought-machine/please/src/core" "github.com/thought-machine/please/src/fs" "io/ioutil" "os" diff --git a/tools/build_langserver/langserver/utils_test.go b/tools/build_langserver/langserver/utils_test.go index b4819d7803..7f23b9f38f 100644 --- a/tools/build_langserver/langserver/utils_test.go +++ b/tools/build_langserver/langserver/utils_test.go @@ -2,8 +2,8 @@ package langserver import ( "context" - "github.com/thought-machine/please/src/core" "fmt" + "github.com/thought-machine/please/src/core" "path" "strings" "testing" diff --git a/tools/build_langserver/langserver_main.go b/tools/build_langserver/langserver_main.go index a3d33f682d..1fde35e28a 100644 --- a/tools/build_langserver/langserver_main.go +++ b/tools/build_langserver/langserver_main.go @@ -2,13 +2,14 @@ package main import ( "context" - "github.com/thought-machine/please/src/cli" - "github.com/thought-machine/please/tools/build_langserver/langserver" "net" "os" "github.com/sourcegraph/jsonrpc2" "gopkg.in/op/go-logging.v1" + + "github.com/thought-machine/please/src/cli" + "github.com/thought-machine/please/tools/build_langserver/langserver" ) // TODO(bnmetrics): also think about how we can implement this with .build_defs as well diff --git a/tools/build_langserver/lsp/specs.go b/tools/build_langserver/lsp/specs.go index 0e748d0454..7f7d55cc5e 100644 --- a/tools/build_langserver/lsp/specs.go +++ b/tools/build_langserver/lsp/specs.go @@ -125,8 +125,8 @@ type MarkedString struct { // MarkupContent represents a string value which content can be represented in different formats. type MarkupContent struct { - Kind MarkupKind `json:"kind"` - Value string `json:"value"` + Kind MarkupKind `json:"kind"` + Value string `json:"value"` } // MarkupKind Describes the content type that a client supports in various result literals diff --git a/tools/build_langserver/lsp/text_document.go b/tools/build_langserver/lsp/text_document.go index 473b97dedd..388a9c8ae3 100644 --- a/tools/build_langserver/lsp/text_document.go +++ b/tools/build_langserver/lsp/text_document.go @@ -82,7 +82,7 @@ type TextDocumentItem struct { // TextDocumentClientCapabilities define capabilities the editor / tool provides on text documents. // TODO: work out if this is being dynamically filled in type TextDocumentClientCapabilities struct { - Completion Completion `json:"completion, omitempty"` + Completion Completion `json:"completion,omitempty"` } // Completion is Capabilities specific to the `textDocument/completion`, referenced in TextDocumentClientCapabilities @@ -112,5 +112,5 @@ type Completion struct { */ CompletionItemKind struct { ValueSet []CompletionItemKind `json:"valueSet,omitempty"` - } `json:"completionItem,omitempty"` + } `json:"completionItemKind,omitempty"` } diff --git a/tools/jarcat/unzip/unzip.go b/tools/jarcat/unzip/unzip.go index 66159d6e28..ac96e4c3fc 100644 --- a/tools/jarcat/unzip/unzip.go +++ b/tools/jarcat/unzip/unzip.go @@ -10,7 +10,7 @@ import ( "strings" "sync" - "third_party/go/zip" + "github.com/thought-machine/please/third_party/go/zip" ) // concurrency controls the maximum level of concurrency we'll allow. diff --git a/tools/jarcat/zip/writer.go b/tools/jarcat/zip/writer.go index b044bb3ffe..f07154c44c 100644 --- a/tools/jarcat/zip/writer.go +++ b/tools/jarcat/zip/writer.go @@ -17,7 +17,7 @@ import ( "gopkg.in/op/go-logging.v1" "github.com/thought-machine/please/src/fs" - "third_party/go/zip" + "github.com/thought-machine/please/third_party/go/zip" ) var log = logging.MustGetLogger("zip") diff --git a/tools/misc/BUILD b/tools/misc/BUILD index 8325809ac3..8b59917a2a 100644 --- a/tools/misc/BUILD +++ b/tools/misc/BUILD @@ -34,7 +34,7 @@ sh_binary( main = "buildify.sh", ) -python_binary( +sh_binary( name = "lint", - main = "ci_lint.py", + main = "lint.sh", ) diff --git a/tools/misc/ci_lint.py b/tools/misc/ci_lint.py deleted file mode 100755 index 254bb0f56d..0000000000 --- a/tools/misc/ci_lint.py +++ /dev/null @@ -1,83 +0,0 @@ -#!/usr/bin/env python3 -"""Implements a bunch of linters on the repo's source. - -Some specific things are run as plz tests (e.g. lint_builtin_rules_test) -but this covers more nebulous things like linters as part of the CI build. - -Should be run from the repo root. -""" - -import os -import subprocess -import sys -from concurrent import futures -from itertools import groupby - - -# Dir names that are always blacklisted. -BLACKLISTED_DIRS = {'plz-out', 'test', 'test_data', 'third_party'} -# Count of packages linted so far and total to do -done = 0 -total = 0 - - -class Linter: - - def __init__(self, root): - all_go_files = [(d, f) for d, f in self.find_go_files(root)] - self.by_dir = [[os.path.join(d, f[1]) for f in files] - for d, files in groupby(all_go_files, key=lambda x: x[0])] - self.done = 0 - self.write('Linting... 0 / %d done', len(self.by_dir)) - - def write(self, msg, args, overwrite=False): - if sys.stderr.isatty() and not os.environ.get('CI'): - if overwrite: - sys.stderr.write('\033[1G') - sys.stderr.write(msg % args) - sys.stderr.flush() - - def reset_line(self): - if sys.stderr.isatty() and not os.environ.get('CI'): - sys.stderr.write('\033[1G') - - def find_go_files(self, root): - for dirname, dirs, files in os.walk(root): - # Excludes github.com, gopkg.in, etc. - dirs[:] = [d for d in dirs if '.' not in d and d not in BLACKLISTED_DIRS] - yield from [(dirname[2:], file) for file in files - if file.endswith('.go') and not file.endswith('.bindata.go')] - - def run_linters(self, files): - try: - # There are two cases of "possible misuse of unsafe.Pointer" in the parser. - # We *think* our usage is legit although of course it would be nice to fix - # the warnings regardless. For now we disable it to get linting of the rest - # of the package. - subprocess.check_call(['go', 'tool', 'vet', '-unsafeptr=false', '-structtags=false'] + files) - subprocess.check_call(['golint', '-set_exit_status'] + files) - return True - except subprocess.CalledProcessError: - return False - finally: - self.done += 1 - self.write('Linting... %d / %d done', (self.done, len(self.by_dir)), overwrite=True) - - -def main(): - linter = Linter('.') - with futures.ThreadPoolExecutor(max_workers=6) as executor: - if not all(executor.map(linter.run_linters, linter.by_dir)): - linter.reset_line() - sys.exit('Some linters failed') - linter.reset_line() - # Run Buildifier - try: - subprocess.check_call(['plz-out/bin/src/please', 'run', '//tools/misc:buildify', '-p', '--', '--mode=check'], - stdout=sys.stdout, stderr=sys.stderr) - except subprocess.CalledProcessError: - sys.exit('Your BUILD files are not correctly formatted, run `plz buildify` to fix.') - - -if __name__ == '__main__': - main() diff --git a/tools/misc/lint.sh b/tools/misc/lint.sh new file mode 100644 index 0000000000..fef9c40399 --- /dev/null +++ b/tools/misc/lint.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash + +plz="plz-out/bin/src/please" +BLACKLIST="src/parse/asp/main|tools/cache|//test|tools/please_pex|third_party" + +# gofmt and go vet +for TARGET in `$plz query alltargets --include go_src --hidden | grep -v "_test#lib" | grep -v proto | grep -Ev $BLACKLIST`; do + DIR=`echo $TARGET | cut -f 1 -d ':' | cut -c 3-` + SRCS=`$plz query print $TARGET -f srcs | grep -v // | while read SRC; do echo $DIR/$SRC; done` + FILTERED=`plz-out/bin/tools/please_go_filter/please_go_filter -tags bootstrap $SRCS` + if [ "$FILTERED" != "" ]; then + go vet -tags bootstrap $FILTERED || { + echo Failed: go vet -tags bootstrap $FILTERED + exit 1 + } + golint --set_exit_status $FILTERED || { + echo Failed: golint --set_exit_status $FILTERED + exit 1 + } + if [ "`gofmt -s -l $FILTERED`" != "" ]; then + echo "Files are not gofmt'd: gofmt -s -l" $FILTERED + exit 1 + fi + fi +done + +$plz run //tools/misc:buildify -p -- --mode=check || { + echo "BUILD files are not correctly formatted; run plz buildify to fix." + exit 1 +}