From e9e616dbd7b4933485f24bf9a76872be07e3a58f Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Thu, 26 Sep 2019 18:19:36 +0200 Subject: [PATCH 1/7] Simplify the 'GolangCILint' class When using a recent version of 'golangci-lint', the generated output is predictable enough to be parsed by a regex instead of converting JSON output. This simplifies the code needed for linting considerably and removes the need of a temporary file. Furthermore, by using the 'filename' capture group introduced in SublimeLinter 4.13, a lot of false positives are fixed. --- README.md | 19 ---- linter.py | 274 +++--------------------------------------------------- 2 files changed, 15 insertions(+), 278 deletions(-) diff --git a/README.md b/README.md index bb268b1..29fbad4 100644 --- a/README.md +++ b/README.md @@ -18,25 +18,6 @@ In order for `golangci-lint` to be executed by SublimeLinter, you must ensure th Due to performance issues in golangci-lint, the linter will not attempt to lint more than one-hundred (100) files considering a delay of 100ms and `lint_mode` equal to “background”. If the user increases the delay, the tool will have more time to scan more files and analyze them. If your project contains more than 300 files, you’ll have to set a delay of 0.3s or more in SublimeLinter settings. -**Note:** The linter creates a temporary directory to allow SublimeLinter to scan changes in the code that are still in the buffer _(aka. not saved yet)_. If the SublimeText sidebar is visible, you will notice _—for a split of a second—_ that a folder named `.golangcilint-*` appears and disappears. Make sure to add this folder to your `.gitignore` file, and also the “folder_exclude_patterns” in SublimeText’s preferences: - -``` -{ - "folder_exclude_patterns": - [ - ".svn", - ".git", - ".hg", - "CVS", - "cache", - "uploads", - ".golangci-*", - ".golangcilint-*", - ".gometalinter-*" - ] -} -``` - ## Plugin installation Please use [Package Control](https://packagecontrol.io/) to install the linter plugin. This will ensure that the plugin will be updated when new versions are available. If you want to install from source so you can modify the source code, you probably know what you are doing so we won’t cover that here. diff --git a/linter.py b/linter.py index 2dae270..3547f1d 100644 --- a/linter.py +++ b/linter.py @@ -1,259 +1,15 @@ -import os -import json -import logging -import tempfile -from SublimeLinter.lint import NodeLinter, util -from SublimeLinter.lint.linter import LintMatch -from SublimeLinter.lint.persist import settings - -logger = logging.getLogger('SublimeLinter.plugin.golangcilint') - - -class Golangcilint(NodeLinter): - # Here are the statistics of how fast the plugin reports the warnings and - # errors via golangci-lint when all the helpers are disabled and only the - # specified linter is enabled. In total, when all of them are enabled, it - # takes an average of 1.6111 secs in a project with seventy-four (74) Go - # files, 6043 lines (4620 code + 509 comments + 914 blanks). - # - # | Seconds | Linter | - # |---------|-------------| - # | 0.7040s | goconst | - # | 0.7085s | nakedret | - # | 0.7172s | gocyclo | - # | 0.7337s | prealloc | - # | 0.7431s | scopelint | - # | 0.7479s | ineffassign | - # | 0.7553s | golint | - # | 0.7729s | misspell | - # | 0.7733s | gofmt | - # | 0.7854s | dupl | - # | 1.2574s | varcheck | - # | 1.2653s | errcheck | - # | 1.3052s | gocritic | - # | 1.3078s | typecheck | - # | 1.3131s | structcheck | - # | 1.3140s | maligned | - # | 1.3159s | unconvert | - # | 1.3598s | depguard | - # | 1.3678s | deadcode | - # | 1.3942s | govet | - # | 1.4565s | gosec | - cmd = "golangci-lint run --fast --out-format json" - defaults = {"selector": "source.go"} - error_stream = util.STREAM_BOTH - line_col_base = (1, 1) - - def run(self, cmd, code): - if not os.path.dirname(self.filename): - logger.warning("cannot lint unsaved Go (golang) files") - self.notify_failure() - return "" - - # If the user has configured SublimeLinter to run in background mode, - # the linter will be unable to show warnings or errors in the current - # buffer until the user saves the changes. To solve this problem, the - # plugin will create a temporary directory, then will create symbolic - # links of all the files in the current folder, then will write the - # buffer into a file, and finally will execute the linter inside this - # directory. - # - # Note: The idea to execute the Foreground linter “on_load” even if - # “lint_mode” is set to “background” cannot be taken in consideration - # because of the following scenario: - # - # - User makes changes to a file - # - The editor suddently closes - # - Buffer is saved for recovery - # - User opens the editor again - # - Editor loads the unsaved file - # - Linter runs in an unsaved file - if settings.get("lint_mode") == "background": - return self._background_lint(cmd, code) - else: - return self._foreground_lint(cmd) - - """match regex against the command output""" - def find_errors(self, output): - current = os.path.basename(self.filename) - exclude = False - - try: - data = json.loads(output) - except Exception as e: - logger.warning(e) - self.notify_failure() - - """merge possible stderr with issues""" - if (data - and "Report" in data - and "Error" in data["Report"]): - for line in data["Report"]["Error"].splitlines(): - if line.count(":") < 3: - continue - if line.startswith("typechecking error: "): - line = line[20:] - if line[1:3] == ":\\": # windows path in filename - parts = line.split(":", 4) - data["Issues"].append({ - "FromLinter": "typecheck", - "Text": parts[4].strip(), - "Pos": { - "Filename": ':'.join(parts[0:2]), - "Line": parts[2], - "Column": parts[3], - } - }) - else: - parts = line.split(":", 3) - data["Issues"].append({ - "FromLinter": "typecheck", - "Text": parts[3].strip(), - "Pos": { - "Filename": parts[0], - "Line": parts[1], - "Column": parts[2], - } - }) - - """find relevant issues and yield a LintMatch""" - if data and "Issues" in data: - for issue in data["Issues"]: - """fix 3rd-party linter bugs""" - issue = self._formalize(issue) - - """detect broken canonical imports""" - if ("code in directory" in issue["Text"] - and "expects import" in issue["Text"]): - issue = self._canonical(issue) - yield self._lintissue(issue) - exclude = True - continue - - """ignore false positive warnings""" - if (exclude - and "could not import" in issue["Text"] - and "missing package:" in issue["Text"]): - continue - - """issues found in the current file are relevant""" - if self._shortname(issue) != current: - continue - - yield self._lintissue(issue) - - def on_stderr(self, output): - logger.warning('{} output:\n{}'.format(self.name, output)) - self.notify_failure() - - def finalize_cmd(self, cmd, context, at_value='', auto_append=False): - """prevents SublimeLinter from appending an unnecessary file""" - return cmd - - def _foreground_lint(self, cmd): - return self.communicate(cmd) - - def _background_lint(self, cmd, code): - folder = os.path.dirname(self.filename) - things = [f for f in os.listdir(folder) if f.endswith(".go")] - maxsee = settings.get("delay") * 1000 - nfiles = len(things) - - if nfiles > maxsee: - # Due to performance issues in golangci-lint, the linter will not - # attempt to lint more than one-hundred (100) files considering a - # delay of 100ms and lint_mode equal to “background”. If the user - # increases the delay, the tool will have more time to scan more - # files and analyze them. - logger.warning("too many Go (golang) files ({})".format(nfiles)) - self.notify_failure() - return "" - - try: - """create temporary folder to store the code from the buffer""" - with tempfile.TemporaryDirectory(dir=folder, prefix=".golangcilint-") as tmpdir: - for filepath in things: - target = os.path.join(tmpdir, filepath) - filepath = os.path.join(folder, filepath) - """create symbolic links to non-modified files""" - if os.path.basename(target) != os.path.basename(self.filename): - os.link(filepath, target) - continue - """write the buffer into a file on disk""" - with open(target, 'wb') as w: - if isinstance(code, str): - code = code.encode('utf8') - w.write(code) - """point command to the temporary folder""" - return self.communicate(cmd + [tmpdir]) - except FileNotFoundError: - logger.warning("cannot lint non-existent folder “{}”".format(folder)) - self.notify_failure() - return "" - except PermissionError: - logger.warning("cannot lint private folder “{}”".format(folder)) - self.notify_failure() - return "" - - def _formalize(self, issue): - """some linters return numbers as string""" - if not isinstance(issue["Pos"]["Line"], int): - issue["Pos"]["Line"] = int(issue["Pos"]["Line"]) - if not isinstance(issue["Pos"]["Column"], int): - issue["Pos"]["Column"] = int(issue["Pos"]["Column"]) - return issue - - def _shortname(self, issue): - """find and return short filename""" - return os.path.basename(issue["Pos"]["Filename"]) - - def _severity(self, issue): - """consider /dev/stderr as errors and /dev/stdout as warnings""" - return "error" if issue["FromLinter"] == "typecheck" else "warning" - - def _canonical(self, issue): - mark = issue["Text"].rfind("/") - package = issue["Text"][mark+1:-1] - # Go 1.4 introduces an annotation for package clauses in Go source that - # identify a canonical import path for the package. If an import is - # attempted using a path that is not canonical, the go command will - # refuse to compile the importing package. - # - # When the linter runs, it creates a temporary directory, for example, - # “.golangcilint-foobar”, then creates a symbolic link for all relevant - # files, and writes the content of the current buffer in the correct - # file. Unfortunately, canonical imports break this flow because the - # temporary directory differs from the expected location. - # - # The only way to deal with this for now is to detect the error, which - # may as well be a false positive, and then ignore all the warnings - # about missing packages in the current file. Hopefully, the user has - # “goimports” which will automatically resolve the dependencies for - # them. Also, if the false positives are not, the programmer will know - # about the missing packages during the compilation phase, so it’s not - # a bad idea to ignore these warnings for now. - # - # See: https://golang.org/doc/go1.4#canonicalimports - return { - "FromLinter": "typecheck", - "Text": "cannot lint package “{}” due to canonical import path".format(package), - "Replacement": issue["Replacement"], - "SourceLines": issue["SourceLines"], - "Level": "error", - "Pos": { - "Filename": self.filename, - "Offset": 0, - "Column": 0, - "Line": 1 - } - } - - def _lintissue(self, issue): - return LintMatch( - match=issue, - message=issue["Text"], - error_type=self._severity(issue), - line=issue["Pos"]["Line"] - self.line_col_base[0], - col=issue["Pos"]["Column"] - self.line_col_base[1], - code=issue["FromLinter"] - ) +from SublimeLinter.lint import Linter, WARNING + + +class GolangCILint(Linter): + cmd = 'golangci-lint run --fast' + # Column reporting is optional and not provided by all linters. + # Issues reported by the 'typecheck' linter are treated as errors, + # because they indicate code that won't compile. All other linter issues + # are treated as warnings. + regex = r'^(?P.[^:]+):(?P\d+):((?P\d+):)? ' + \ + r'(?P.+\(((?Ptypecheck)|\w+)\))$' + default_type = WARNING + defaults = { + 'selector': 'source.go' + } From 11e78c166defbe3b1c5ce30e906b295f79acec73 Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Thu, 26 Sep 2019 18:35:07 +0200 Subject: [PATCH 2/7] Support absolute paths with drive names on Windows --- linter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linter.py b/linter.py index 3547f1d..9d751f2 100644 --- a/linter.py +++ b/linter.py @@ -7,7 +7,7 @@ class GolangCILint(Linter): # Issues reported by the 'typecheck' linter are treated as errors, # because they indicate code that won't compile. All other linter issues # are treated as warnings. - regex = r'^(?P.[^:]+):(?P\d+):((?P\d+):)? ' + \ + regex = r'^(?P(\w+:\\\\)?.[^:]+):(?P\d+):((?P\d+):)? ' + \ r'(?P.+\(((?Ptypecheck)|\w+)\))$' default_type = WARNING defaults = { From 942a6d5dd52b283f9169b15c1f2f6c0ad3adcf72 Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Mon, 30 Sep 2019 09:17:26 +0200 Subject: [PATCH 3/7] Use 'tab' output format as regex input. This also sets the 'code' capture group. --- linter.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/linter.py b/linter.py index 9d751f2..c18fb19 100644 --- a/linter.py +++ b/linter.py @@ -2,13 +2,13 @@ class GolangCILint(Linter): - cmd = 'golangci-lint run --fast' + cmd = 'golangci-lint run --fast --out-format tab' # Column reporting is optional and not provided by all linters. # Issues reported by the 'typecheck' linter are treated as errors, # because they indicate code that won't compile. All other linter issues # are treated as warnings. - regex = r'^(?P(\w+:\\\\)?.[^:]+):(?P\d+):((?P\d+):)? ' + \ - r'(?P.+\(((?Ptypecheck)|\w+)\))$' + regex = r'^(?P(\w+:\\\\)?.[^:]+):(?P\d+)(:(?P\d+))?\s+' + \ + r'(?P(?Ptypecheck)|\w+)\s+(?P.+)$' default_type = WARNING defaults = { 'selector': 'source.go' From 73ab429109dad9af8fa0538b05df2a69c269a0d0 Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Mon, 30 Sep 2019 09:18:24 +0200 Subject: [PATCH 4/7] Mark this linter as 'file-only' --- linter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/linter.py b/linter.py index c18fb19..3f62cec 100644 --- a/linter.py +++ b/linter.py @@ -9,6 +9,7 @@ class GolangCILint(Linter): # are treated as warnings. regex = r'^(?P(\w+:\\\\)?.[^:]+):(?P\d+)(:(?P\d+))?\s+' + \ r'(?P(?Ptypecheck)|\w+)\s+(?P.+)$' + tempfile_suffix = '-' default_type = WARNING defaults = { 'selector': 'source.go' From 4b2e3eda5f2f4ab32f364e90169348a01e8f740c Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Mon, 30 Sep 2019 11:43:04 +0200 Subject: [PATCH 5/7] Revert "Mark this linter as 'file-only'" This reverts commit 73ab429109dad9af8fa0538b05df2a69c269a0d0. --- linter.py | 1 - 1 file changed, 1 deletion(-) diff --git a/linter.py b/linter.py index 3f62cec..c18fb19 100644 --- a/linter.py +++ b/linter.py @@ -9,7 +9,6 @@ class GolangCILint(Linter): # are treated as warnings. regex = r'^(?P(\w+:\\\\)?.[^:]+):(?P\d+)(:(?P\d+))?\s+' + \ r'(?P(?Ptypecheck)|\w+)\s+(?P.+)$' - tempfile_suffix = '-' default_type = WARNING defaults = { 'selector': 'source.go' From 8d844ba03fc4668f36d1d7ff2b31c84c10f5cd04 Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Mon, 7 Oct 2019 09:47:55 +0200 Subject: [PATCH 6/7] Lint all files in a directory to avoid false positives. Go packages can consist of multiple files in the same folder. Because constants, types, ... are shared between these files of a package, the whole folder must be linted to avoid false positives. --- linter.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/linter.py b/linter.py index c18fb19..0922c94 100644 --- a/linter.py +++ b/linter.py @@ -2,7 +2,8 @@ class GolangCILint(Linter): - cmd = 'golangci-lint run --fast --out-format tab' + cmd = 'golangci-lint run --fast --out-format tab ${file_path}' + tempfile_suffix = '-' # Column reporting is optional and not provided by all linters. # Issues reported by the 'typecheck' linter are treated as errors, # because they indicate code that won't compile. All other linter issues From a0b03158f1dba24e2d07acf2cd1d2923ec95aa2a Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Wed, 9 Oct 2019 09:50:19 +0200 Subject: [PATCH 7/7] Clarify linting behavior. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 29fbad4..9397a0c 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ This linter plugin for [SublimeLinter](https://github.com/SublimeLinter) provide In order for `golangci-lint` to be executed by SublimeLinter, you must ensure that its path is available to SublimeLinter. Before going any further, please read and follow the steps in [Finding a linter executable](http://sublimelinter.readthedocs.org/en/latest/troubleshooting.html#finding-a-linter-executable) through “Validating your PATH” in the documentation. Once you have installed `golangci-lint`, you can proceed to install the plugin if it is not yet installed. -Due to performance issues in golangci-lint, the linter will not attempt to lint more than one-hundred (100) files considering a delay of 100ms and `lint_mode` equal to “background”. If the user increases the delay, the tool will have more time to scan more files and analyze them. If your project contains more than 300 files, you’ll have to set a delay of 0.3s or more in SublimeLinter settings. +Due to the way that golangci-lint works, the linter will only run when saving a file, even if `lint_mode` is set to “background”. ## Plugin installation