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

Feature request: the ability to specify specific paths #18

Open
Antonboom opened this issue Aug 19, 2022 · 6 comments
Open

Feature request: the ability to specify specific paths #18

Antonboom opened this issue Aug 19, 2022 · 6 comments

Comments

@Antonboom
Copy link

Hello!

To disallow the use of something in specific packages, not for the entire project.

@ashanbrown
Copy link
Owner

Hi @Antonboom , are you using golangci-lint? That's how I would recommend using this tool. The docs for golangcilint issues configuration at https://golangci-lint.run/usage/configuration/#issues-configuration describe how you could exclude rules for specific paths. I've been reluctant to implement a detailed confiugration solution just for this linter (since it's a problem every linter has).

@Antonboom
Copy link
Author

Antonboom commented Aug 21, 2022

@ashanbrown, the problem is that golangci-lint allows you to exclude rules but not include them :)

Like:

forbidigo:
    paths: internal/services
    forbid: [ "sql.Tx" ]

@ashanbrown
Copy link
Owner

I've been looking to see if there is any discussion of this on golangci-lint. I did find this discussion at golangci/golangci-lint#420. but it looks like it didn't go anywhere. I also raised a somewhat similar issue myself at golangci/golangci-lint#2440, ultimately proposing a notpath directive, whic hdidn't get much traction.

Out of curiosity, are you genuinely using this as a standalone linter? I've kind of assumed no one is, if only for efficiency's sake, so I'm reluctant to build something linter specific.

One option could be to extend the pattern format to include a path. We already have a sort of hacky comment notation:

^fmt\.Errorf(# please use github\.com/pkg/errors)?$ -- forbid Errorf, with a custom message

The discussion that led to this happened at #11. So perhaps, we could again do some sort path buried in the regex, maybe like:

^sql\.Tx(@^internal/services/)?$

I haven't gone done the path of adding detailed per-pattern rule configuration to this linter because I'd like it to support the golang analyzer interface, which passes data through golang flags. I'm not quite sure how I'd get the data though a flag (I suppose we could parse just json from a string and have golangci-lint generate that json).

Let me know if you have any thoughts about the best approach.

@Antonboom
Copy link
Author

Antonboom commented Aug 22, 2022

Out of curiosity, are you genuinely using this as a standalone linter?

No, I use golangci-lint.

Let me know if you have any thoughts about the best approach.

I think the best solution would be to support include-rules (opposite existing exclude-rules) in golangci-lint. But I'm not sure if they'll go for it :(

@pohly
Copy link
Contributor

pohly commented Oct 20, 2022

I also need something like this for Kubernetes.

I am removing the usage of certain functions from test/e2e* directories inside the main Kubernetes repo because they don't accept a context and thus block when someone tries to abort a test run or the test times out. Without a linter, it is very likely that developers who don't know about this will add back code using those functions and get that accepted because maintainers also still need to learn about this.

@pohly
Copy link
Contributor

pohly commented Oct 27, 2022

See #22 for a proposed solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants