Support lint for sub-package usage #2
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The given NOT to import package will be linted not only for itself, but also for its sub-packages. The lint message will also reflect this. A potential counterpart change would be to merge the input argument and merge them if they has root-sub relationship. Or we should allow user to pass in two paths, e.g. *a=x* and *a/b=y*, and lint the import path of *a/b* using the second hint (i.e. use suggestion of *y*), this allows package author to split a mono package into several independent packages.
fatih
reviewed
Mar 6, 2020
faillint/faillint.go
Outdated
| @@ -52,15 +52,30 @@ func run(pass *analysis.Pass) (interface{}, error) { | |||
|
|
|||
| for _, file := range pass.Files { | |||
| for _, path := range imports { | |||
| imp := usesImport(file, path) | |||
| imp, exact := usesImport(file, path) | |||
There was a problem hiding this comment.
I'm not sure about the "exact" word here. What do you think about "fullMatch" or "exactMatch" ? Or you reverse it and return the "subMatch" (or "subPath"). Then below you could say if subMatch { ... } which I think is easier to read and reason than if !exact { ... }
There was a problem hiding this comment.
Thank you for the comment, I agree!
fatih
added a commit
that referenced
this pull request
Mar 31, 2020
fatih
added a commit
that referenced
this pull request
Mar 31, 2020
fatih
added a commit
that referenced
this pull request
Apr 1, 2020
fatih
added a commit
that referenced
this pull request
Apr 1, 2020
|
This is now fixed with the change here: #14 I'm sorry that we couldn't get this in, but we had my changes so we had to implement it in a different way. I appreciate your work and let me know if you see any problems. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The given NOT to import package will be linted not only for itself, but also for its sub-packages. The lint message will also reflect this.
Additionally we should allow user to pass in root package path and sub package path and prefer the exact matching path if any. E.g. when pass in a=x and a/b=y, then we should lint the import path of a/b using the a/b=y suggestion (instead of a=x), this allows package author to split a mono package into several independent packages.
Another potential issue is that user might passing, e.g., a, a/b, and the code actually only import a/b/c. For now I didn't considered this case yet since I want to see whether I'm in the correct direction first, before diving into too detail.