ashanbrown / forbidigo

Go linter for forbidding identifiers
Other
118 stars 10 forks source link

Feature request: the ability to specify specific paths #18

Open Antonboom opened 1 year ago

Antonboom commented 1 year ago

Hello!

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

ashanbrown commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

I've been looking to see if there is any discussion of this on golangci-lint. I did find this discussion at https://github.com/golangci/golangci-lint/issues/420. but it looks like it didn't go anywhere. I also raised a somewhat similar issue myself at https://github.com/golangci/golangci-lint/issues/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 https://github.com/ashanbrown/forbidigo/pull/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

See https://github.com/ashanbrown/forbidigo/pull/22 for a proposed solution.