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

go-errcheck checker doesn't support multi-element $GOPATH #580

Closed
bdarnell opened this issue Feb 4, 2015 · 2 comments
Closed

go-errcheck checker doesn't support multi-element $GOPATH #580

bdarnell opened this issue Feb 4, 2015 · 2 comments

Comments

@bdarnell
Copy link

bdarnell commented Feb 4, 2015

The GOPATH environment variable can be a colon-separated list of paths, which makes the mapping from filename to go package non-trivial (furthermore, in one of my projects where this is an issue, one GOPATH element is a prefix of another (github.com/cockroachdb/cockroach). Ideally the longest match should be used instead of the first match, although I haven't run into a case where this matters). flycheck-go-package-name needs to be augmented to support this; here is my attempt:

(defun flycheck-go-package-name (&optional file-name gopath)
  "Determine the package name for FILE-NAME and GOPATH.

FILE-NAME defaults to `buffer-file-name'.  GOPATH defaults to
$GOPATH.

Return the package name for FILE-NAME, or nil if FILE-NAME is not
part of any package or if GOPATH is nil."
  (-when-let* ((gopath (or gopath (getenv "GOPATH")))
               (file-name (or file-name (buffer-file-name))))
    (--min-by (> (length it) (length other))
     (-non-nil
      (--map
       (let ((gosrc (file-name-as-directory (expand-file-name "src/" it)))
             (file-name (expand-file-name file-name)))
         (when (string-prefix-p gosrc file-name)
           ;; The file is part of a package, so determine the package name, as
           ;; relative file name to the GO source directory
           (directory-file-name            ; Remove trailing /
            (file-relative-name (file-name-directory file-name) gosrc))))
       (split-string gopath ":"))))))

Alternately, we could just call out to the go tool: go list $DIR prints the package name defined by the given directory. The error-filter also needs to be updated to reassemble the path based on the prefix that was chosen by flycheck-go-package-name.

See also kisielk/errcheck#45, which would hopefully make all of this obsolete by making errcheck work more like the other go tools.

@swsnr
Copy link
Contributor

swsnr commented Feb 5, 2015

@bdarnell Feel free to open a pull request. I won't change the corresponding code by myself, though: The current implementation is just a best guess, which tries to handle the simple case well. I don't know enough about Go—nothing actually—to properly handle $GOPATH.

@swsnr swsnr modified the milestone: community backlog Apr 6, 2015
@swsnr swsnr removed the needs-patch label Apr 6, 2015
@swsnr
Copy link
Contributor

swsnr commented Aug 1, 2015

Closing as wontfix, because I'll not fix this myself. I do not use Go, and don't know anything about it.

That said, pull requests and patches are welcome, and I'll be happy to review and merge anything that addresses this issue.

@swsnr swsnr closed this as completed Aug 1, 2015
swsnr pushed a commit that referenced this issue Mar 17, 2016
The -abspath flag was introduced in commit
8515d34a8746fc0ab1c652594faaeed138694c49 on the 25th of August, 2015. It
causes errcheck to print absolute file paths.

This change simplifies the code and makes it work correctly with
multiple GOPATH entries.

Closes GH-906 and fixes GH-580.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants