[WIP] Integration of FSharpLint

[duckmatt]

This pull request is to finish up the intgeration of FSharpLint by providing a configuration pane in the power tools options. #1094

The work here so far is mostly to display an existing (default) configuration file in the options. Going forwards I'm looking to integrate it with the configuration management functionality I've been adding to FSharpLint so that any updates made by the user are saved back to disk, and then cleaned up into a state where it can be merged.

[dungpa]

Great jobs. I think it needs to be hooked with config write API then we're done.

Time for some screenshots:

[dungpa]

Is this possible to open ConfigurationManagement?

[vasily-kirichenko]

member __.Name = name?

[duckmatt]

Yep, it tries to save the settings to the solution directory by default (and these settings can be overridden with settings files in project directories for project specific settings)

[duckmatt]

There's now a dialog that lets the user retry or cancel a save when their save fails. The failing test is now failing with:

Attempt by method 'FSharpLint.Framework.HintParser+Identifiers+pidenttext@118-1.Invoke(Microsoft.FSharp.Collections.FSharpList`1<Char>)' 
to access method 'Microsoft.FSharp.Compiler.Lexhelp+Keywords.get_keywordNames()' failed.

I'll try to release a new version of the FSharpLint api in line with the latest FSharp.Compiler.Service to fix this

[dungpa]

The failing test has been fixed.

@vasily-kirichenko Please merge this PR if you think it's ready.

[vasily-kirichenko]

I'm not sure what does it means:

The group is selected, but no one child is. It's confusing.
Either this one:

As the group is not checked, I'm not sure the selected child will work.
My propose: when user checks a group, all children should be checked automatically. And vise versa: if you uncheck a group, then all the children should be unchecked. It opens a new question: what state should a group be in if some children are checked, but others are not. It seems a common solution is to introduce the third group state for this:

[vasily-kirichenko]

If I want to disable some rule, it's hard to find the setting related to it. For example, I've no idea how to switch it off:

[vasily-kirichenko]

It would be more comfortable if the checkbox were checked/unchecked by clicking on "Enabled" label, too:

[vasily-kirichenko]

1. Disable some rule that's enabled by default.
2. The config file has the settings off:

<?xml version="1.0" encoding="utf-8"?>
<FSharpLintSettings xmlns="https://github.com/fsprojects/FSharpLint/blob/master/ConfigurationSchema.xsd">
  <IgnoreFiles Update="Overwrite"><![CDATA[assemblyinfo.*]]></IgnoreFiles>
  <Analysers>
    <Binding>
      <Rules>
        <WildcardNamedWithAsPattern>
          <Enabled>True</Enabled>
        </WildcardNamedWithAsPattern>
      </Rules>
    </Binding>
    <Hints>
      <Rules />
      <Enabled>False</Enabled>
    </Hints>
  </Analysers>
</FSharpLintSettings>

1. Turn the rule on back.
2. The settings file contains the settings with default value:

<?xml version="1.0" encoding="utf-8"?>
<FSharpLintSettings xmlns="https://github.com/fsprojects/FSharpLint/blob/master/ConfigurationSchema.xsd">
  <IgnoreFiles Update="Overwrite"><![CDATA[assemblyinfo.*]]></IgnoreFiles>
  <Analysers>
    <Binding>
      <Rules>
        <WildcardNamedWithAsPattern>
          <Enabled>False</Enabled>
        </WildcardNamedWithAsPattern>
      </Rules>
    </Binding>
    <Hints>
      <Rules />
      <Enabled>False</Enabled>
    </Hints>
  </Analysers>
</FSharpLintSettings>

Expected behavior: default settings should not be stored in the settings file.

[duckmatt]

I agree with all your points.

I think the solution to finding a setting for a warning will be to introduce identifiers for the lint suggestions

The last point is a difficult one, the reason why the default settings are left in a file is in case someone wanted to enforce a default - this would be a rare case where someone could have a default setting changed in a setting file in an ancestor directory. I think as this case is somewhat rare it might be worth not adding default values, and if someone wants that behaviour they can edit the file themselves, what do you think?

[vasily-kirichenko]

OK, I agree about default settings left in the files, let's leave it as it is.

About identifiers for suggestions, it'd work, but not that elegant. A good solution would be to add smart tags, which would include something like "switch this rule off for current project / whole solution", but it requires a lot of work. However, if we plan to add quick actions for suggestions, adding one more action into them would be quite easy.

[dungpa]

I think adding labels for FSharpLint warnings is a good solution.

As a user, I would prefer:

FL0001: Identifiers Must Not Contain Underscores

than

IdentifiersMustNotContainUnderscores

in the setting dialog.

The former is easy to understand and look up for further documentation.

We can always add smart tags support if we get to it later.

[vasily-kirichenko]

@dungpa 👍

[duckmatt]

Great, I'll look at making those UI changes, and adding support to FSharpLint for the rule identifiers

[vasily-kirichenko]

The checkboxes behave better now, but still inconsistently:

[duckmatt]

Thanks for pointing that out - should be sorted now. I was looking at doing the tri-state checkboxes but thinking more about it I don't think it fits perfectly as it's possible for the analysers (the parent checkboxes) to only have a disabled/enabled state in the config file

[vasily-kirichenko]

Great job, Matt! Thanks a lot both for FsLinter and for this integration.

[dungpa]

@duckmatt Thank you for the hard work and patience on this PR.

Would you mind to add a short documentation page with screenshots for the feature https://github.com/fsprojects/VisualFSharpPowerTools/tree/master/docs/content ? It's the last thing we hope to finish before releasing VFPT 2.1.0.

[forki]

Great stuff. Kudos

[vasily-kirichenko]

@dungpa I think we could include the outlining feature as well.

[duckmatt]

Thanks guys, I'll look at getting a doc page together