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.

Sign up for GitHub

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

Jump to bottom

Error when analyzer globs go unmatched #204

Closed
wants to merge 3 commits into from
Closed

Error when analyzer globs go unmatched #204

wants to merge 3 commits into from

Conversation

Smaug123
Copy link
Contributor

@Smaug123 Smaug123 commented Jan 30, 2024 •
edited
Loading

This is a perf hit, of course, but I claim it's not an important one.

I took the liberty of unindenting the code at the end of the main function (by early-exiting), but this has made the diff pretty nasty, sorry :(

Fixes #203.

I could understand not wanting this; on balance, though, I think these cases are more likely to be user error than intended.

nojaf
nojaf approved these changes Jan 31, 2024
View reviewed changes
Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I don't mind the unindent.
I'll let @dawedawe have the final say.

@dawedawe
Copy link
Contributor

How about we change the log level to warning and remove the last log message about aborting.
Considere the different behaviours of a missing Exclude and a missing Include glob:

Include:

dotnet run --project src/FSharp.Analyzers.Cli/FSharp.Analyzers.Cli.fsproj -- --project ./samples/OptionAnalyzer/OptionAnalyzer.fsproj --analyzers-path ./samples/OptionAnalyzer/bin/Release --verbosity d --include-analyzers Foo
info: Running in verbose mode
info: Treat as Hints: []
info: Treat as Info: []
info: Treat as Warning: []
info: Treat as Error: []
info: Exclude Files: []
info: Include Files: []
info: Loading analyzers from /home/dawe/src/FSharp.Analyzers.SDK/samples/OptionAnalyzer/bin/Release
info: Registered 0 analyzers from 1 dlls
error: The following glob(s) were specified to include or exclude specific analyzers, but they did not match any discovered analyzers. Have you got them right? Foo

Exclude:

dotnet run --project src/FSharp.Analyzers.Cli/FSharp.Analyzers.Cli.fsproj -- --project ./samples/OptionAnalyzer/OptionAnalyzer.fsproj --analyzers-path ./samples/OptionAnalyzer/bin/Release --verbosity diag --exclude-analyzers foo
info: Running in verbose mode
info: Treat as Hints: []
info: Treat as Info: []
info: Treat as Warning: []
info: Treat as Error: []
info: Exclude Files: []
info: Include Files: []
info: Loading analyzers from /home/dawe/src/FSharp.Analyzers.SDK/samples/OptionAnalyzer/bin/Release
info: Registered 1 analyzers from 1 dlls
info: Running analyzers for /home/dawe/src/FSharp.Analyzers.SDK/samples/OptionAnalyzer/obj/Debug/net6.0/.NETCoreApp,Version=v6.0.AssemblyAttributes.fs
info: Running analyzers for /home/dawe/src/FSharp.Analyzers.SDK/samples/OptionAnalyzer/Library.fs
info: Running analyzers for /home/dawe/src/FSharp.Analyzers.SDK/samples/OptionAnalyzer/obj/Debug/net6.0/OptionAnalyzer.AssemblyInfo.fs
error: The following glob(s) were specified to include or exclude specific analyzers, but they did not match any discovered analyzers. Have you got them right? foo
/home/dawe/src/FSharp.Analyzers.SDK/samples/OptionAnalyzer/Library.fs(10,4): Warning OV001 - Option.Value shouldn't be used
/home/dawe/src/FSharp.Analyzers.SDK/samples/OptionAnalyzer/Library.fs(21,52): Warning OV001 - Option.Value shouldn't be used
error: Aborting because there were unmatched analyzer globs (see earlier log).

A non-matching Exclude glob means, we still run the analyzers from the given directories as seen in the output above.
Given that, the last log message doesn't sound right to me.

@nojaf
Copy link
Contributor

nojaf commented Feb 27, 2024

Hi @Smaug123, did you see @dawedawe's last message here?

@Smaug123
Copy link
Contributor Author

I don't really want this feature enough to work on it any more, thanks!

@Smaug123 Smaug123 closed this Feb 27, 2024
@Smaug123 Smaug123 deleted the warn-on-unmatched-analyzers branch February 27, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nojaf nojaf nojaf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Have Include/ExcludeFilter warn if an input glob didn't match any analyzers
3 participants
@Smaug123 @dawedawe @nojaf