Allow to configure which diagnostics linting rules are enabled

[aparent-emgs]

Not sure if this already has been asked or if this is already possible, but as I couldn't find anything in the documentation for that, and I couldn't find any similar issues, I thought I'd bring this up. In previous versions in which we could enable elm-analyse, we could use the elm-analyse.json file to configure which rules you wanted to enable/disable for your particular setup, in a format that resembled this one:

{
    "checks": {
        "SingleFieldRecord": false,
        "NoTopLevelSignature": false,
        "MultiLineRecordFormatting": false
    },
    "TriggerWords": {
        "words": []
    }
}

After updating, I've noticed that there is no such option to configure which rules are enabled with the diagnostics that are returned by the language server in the newest version. It's basically a feature that I've lost after updating since the plugin stopped using elm-analyse to prioritise its own language server. I have no problems with that, as I absolutely love the other changes that were made since the last time I had updated, but I think it could be an interesting enhancement to allow the same level of configuration as comparable tools to control which linting rules you don't really care about.

[razzeee]

We do support this in the Form of an elm-analyse.json next to your elm.json

[aparent-emgs]

Okay, then maybe I am mistaken, and the problem is rather that some configurations that used to work with elm-analyse no longer are taken into consideration for the new diagnostics. The JSON snippet I included is the exact same elm-analyse.json we've been using for a while in our project. We used this configuration:

"NoTopLevelSignature": false,

Because in some cases we would define simple records of constants such as this one:

definitionMaxLengths =
    { name = 15
    , description = 45
    , prefix = 1
    , comments = 2000
    }

And we wanted to be able to define these records without having to define a redundant type and without getting the elm-analyse warning for it. Now that we've updated to the newest version of the plugin in VS Code, we're once again getting warnings for this rule.

[razzeee]

We did not implement NoTopLevelSignature from elm-analyse, as it's debatable if this is a good practice. As this rule is global, while it would get rid of your simple case being remarked, it would also not help you with every other case from that point on.

What your seeing at the moment is our own type inference checker.

[aparent-emgs]

Yea, I absolutely get what you mean. We took the decision to remove it as we already had the habit to add the type annotation before even implementing any function in a file, and removing the rule made these cases where we don't actually need it a whole lot less noisy. Nevertheless, I understand perfectly that we are most probably the exception here, and that most people would find no reason to turn that rule off -- I was honestly surprised it is even a configurable rule in elm-analyse.

Since I honestly just thought that the configuration of which rules gets handled or not was simply not implemented, which is finally clearly not the case, I have absolutely no problem if this issue is not handled in high priority, or if it is discarded altogether... as the maintainers of this package see fit. If my team really wants to keep this rule, we can just turn off the diagnostics and continue using elm-analyse as is.

[razzeee]

I tried to sum up the ones we did not end up implementing here https://github.com/elm-tooling/elm-language-server/blob/main/src/providers/diagnostics/elmLsDiagnostics.ts#L29

Feel free to have a look and get back to us. I think we might close this, but would also be interested in some more opinions.