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

Have Include/ExcludeFilter warn if an input glob didn't match any analyzers #203

Open
Smaug123 opened this issue Jan 30, 2024 · 2 comments

Comments

@Smaug123
Copy link
Contributor

Smaug123 commented Jan 30, 2024

How would you feel about having IncludeFilter and ExcludeFilter be Glob list, instead of string -> bool? The FSharp.Analyzers.Cli consumer in this repo does this:

fun (s: string) -> i |> List.map Glob |> List.exists (fun g -> g.IsMatch s)
|> IncludeFilter

But what I would really like is to be able to report "this particular glob was never matched - are you sure you got it right?". That's because when I first tried using --exclude-analyzers, I used the name GRA-PARTAPP-001 rather than the correct PartialAppAnalyzer, and it was sheer coincidence that I happened to be in a repo where that warning was already firing (so I noticed I had failed to suppress it).

(Doing this would require running every glob against every analyzer, but there's presumably not going to be more than 10k analyzers ever, so that isn't going to be too much of a slowdown.)

  • The cleanest way to do this would be to defunctionalise {Include/Exclude}Filter to hold only a list of globs. That way, the analyzer SDK could tally how many times it matched each glob against an analyzer, and report the result. Disadvantages: breaking change which possibly removes functionality downstream consumers were depending on; another dependency (to obtain Glob)
  • An alternative way would be for FSharp.Analyzers.Cli to pass a function which mutates some state, and then check the state at the end. I find that aesthetically displeasing, but it would confine the changes to the CLI rather than requiring a change in the library.

Which of those two would you prefer, if either? (I'm happy to implement either.)

@dawedawe
Copy link
Contributor

Hey,
we are fine with the additional reporting of non-matching globs.
But we shoudln't force the users of FSharp.Analyzers.SDK.Client (Editors like Ionide/FSAC) to take a dependency on Glob.
So if you can do the necessary changes while keeping the SDK API in it's current state, that would be great.

@Smaug123
Copy link
Contributor Author

Cool - I'll make the change only in the CLI tool.

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

Successfully merging a pull request may close this issue.

2 participants