ionide / ionide-analyzers

http://ionide.io/ionide-analyzers/
MIT License
5 stars 7 forks source link

Add analyzers from FSharp.Formatting PR #85

Closed nojaf closed 4 months ago

nojaf commented 4 months ago

In https://github.com/fsprojects/FSharp.Formatting/pull/906, @thorium made some good suggestions to improve the performance of the code.

Some of these checks are interesting to turn into an analyzer I think. I will try and add some analyzers in this PR.

Thorium commented 4 months ago

I have more suggestions if you run out :-D

nojaf commented 4 months ago

I have more suggestions if you run out :-D

Thanks, that is good input. Realistically speaking I'll probably not get to those. My time for this is limited.

Numpsy commented 4 months ago

I'm sure there are plenty of possible ideas for analyzers about (be it borrowing more rules from FSharpLint, or looking at things like NetAnalyzers and Roslynator, or more F# specific stuff) - is it worth creating a more general list of suggestions somewhere to track ideas?

nojaf commented 4 months ago

is it worth creating a more general list of suggestions somewhere to track ideas?

Not to throw shade, but who's actually gonna make it happen, right? 😉 Just forecasting the vibes, not trying to be harsh.

Thorium commented 4 months ago

Something that ionide-analysers could in theory do, and not so much of others (just because they are not F# IDE-plugins), would be these 2 way conversions of "change A to B" and "change B to A" and leave final decision for user to decide. Because that would help refactorings.

nojaf commented 4 months ago

@dawedawe feel free to already start nitpicking if you like.

Numpsy commented 4 months ago

A question about the Add [<Struct>] to DU when it makes sense. suggestion/fix:

Say I have a DU in one of these forms:

type SomeType =
| Success
| SomeError of string
| SomeOtherError of string

type SomeType =
| Success
| SomeError of error: string
| SomeOtherError of error: string

I get the 'make it a struct' suggestion and quick fix: image But if I action the quick fix, I get: image

Maybe making sure the cases have unique names is a good suggestion anyway, but fwiw - should the quick fix make a change that then errors?

nojaf commented 4 months ago

A question about the Add [] to DU when it makes sense. suggestion/fix:

I've noticed the issue as well. Ideally, the quick fix should address this. I'm inclined to accept the quick fix as it stands. While it's not ideal, anyone interested can further refine it with subsequent PRs. I feel this option might be the better choice compared to turning off the code fix and nobody ever touches it again.

This PR's work is supported by the Amplifying F# Open Collective (more details to be shared publicly later this week). We allocated one working day for this task, and we've now surpassed that limit. I'll be shifting my focus to other work that supports my livelihood.