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

Please provide example of how a PLC Analyzer/Diagnostic should get options from user #6234

Closed
paul1956 opened this issue Oct 22, 2015 · 18 comments
Assignees

Comments

@paul1956
Copy link
Contributor

I am making this a standalone request,

Things I am finding difficult;
Getting initial values from file stored in solution
Using Option to load, store the options.
I would be happy with just a single String that I could parse, bonus points for anything fancier.
I would like the Example in VB or translatable to VB.
Please don't use internal API's

@tmeschter
Copy link
Contributor

@paul1956 It sounds like you're looking for more information about how to read Additional Files (that is, non-source code files) from an analyzer. Does this page meet your needs?
https://github.com/dotnet/roslyn/blob/master/docs/analyzers/Using%20Additional%20Files.md

If it doesn't, what could be added to help you out?

Or have I misunderstood the request?

@paul1956
Copy link
Contributor Author

That is exactly what I was looking for except that additionalFiles is nothing. From the image below I would have expected to see WebEssentials2015-Settings.json.

Dim additionalFiles As ImmutableArray(Of AdditionalText) = context.Options.AdditionalFiles

image

@tmeschter
Copy link
Contributor

@paul1956 Analyzers can only see what is in the project; adding a file to Solution Items won't make it show up as an Additional File any more than adding a .cs or .vb file to Solution Items causes it to be built as part of a project.

You'll need to update the actual project file to refer to the settings file, as described in the link.

@paul1956
Copy link
Contributor Author

So there are no, solution wide settings? How does the VSIX project which controls the menu items work with the file. The example only addresses reading the file (which was very helpful) not creating and setting the file. The VSIX is a different project from the Analyzer...

BTW: the markup in the example cuts off the right side of a few of the long code lines (at least using the Edge Browser), Raw does show the whole line.

@tmeschter
Copy link
Contributor

For the purposes of this discussion, I think we need to distinguish analyzers and Code Fixes. They are related, but very different in how they interact with projects and files.

Analyzers are designed to work as part of the build process, and so they only see what the compiler sees: the files, references, and settings specific to a project. The analyzer has no concept of MSBuild or solutions because the compiler has no concept of MSBuild or solutions. The analyzer only has access to the services and information provided by the compiler. Further, files are read-only; there are no facilities for editing files at this level.

Code Fixes, on the other hand, work within the Roslyn Workspace, so they can edit files, work across projects, and also access services provided by the host (for example, Visual Studio).

This leads to a question: what are you doing with this settings file? Does the analyzer need it, or does the Code Fix need it?

If the analyzer needs it then you need to set it up as an Additional File in the project; that's the only supported mechanism for analyzers to read non-source documents.

If only the Code Fix needs it, then you have a lot more options. You could still include it as an Additional File, which can be accessed through Project.AdditionalDocuments. And a Code Fix could update the file in much the same way it updates a source code file (see https://github.com/dotnet/roslyn-analyzers/blob/master/src/Roslyn/Core/ApiDesign/DeclarePublicAPIFix.cs for an example).

Your Code Fix could also use the VS extensibility points to access a file that isn't represented in Roslyn in any way, like a file in Solution Items. Of course, Roslyn can't help you with that, and I won't be able to provide any guidance, either.

The correct approach will depend on exactly how you are using the file in question; I've just laid out the basic options.

@paul1956
Copy link
Contributor Author

The Analyzer needs a read only list of names, references and Namespaces to ignore when reporting an issue in NameOf Analyzer, for example I don't want report an issue with "String". I build the list in the VSIX project by interaction with the user and heuristics now I am asking how to correctly connect the two. Also related is when I change the file (however it happens) will the Analyzer run again?

@paul1956
Copy link
Contributor Author

I am operating at the node level will the example require rereading the file for every node?

@tmeschter
Copy link
Contributor

If you use Additional Files then yes, changes to the file will automatically cause the analyzer to run again.

In the example at https://github.com/dotnet/roslyn/blob/master/docs/analyzers/Using%20Additional%20Files.md, the file is read once in the the compilation start action to produce a list of terms which is then reused in every symbol action. You would want to do something similar.

Note that a new compilation is created on every edit, so in VS you may end up reading the file on every keystroke. Another option would be to read the settings file lazily, the first time you need it.

@sharwell
Copy link
Member

Note that a new compilation is created on every edit, so in VS you may end up reading the file on every keystroke.

Is there a delay between the time when the following occur?

  1. A keystroke is entered
  2. A compilation is created for the modified text after the keystroke
  3. Analyzers are registered (via DiagnosticAnalyzer.Initialize)
  4. Compilation start actions which were registered in the previous step are invoked

Specifically, if a user is typing quickly can/will the process be stopped prior to reaching step 4?

@tmeschter
Copy link
Contributor

@sharwell I was generalizing. In VS there is a delay between when a keystroke is entered and when we create a new compilation. That way a fast typist does not saturate the system with new compilations.

@paul1956
Copy link
Contributor Author

Lazy load in VB is easy, but I still have the issue of when and how to create the default file. There would need to be one configuration file per project in the solution even though the values would be the same, since for all projects I want to ignore the same list of strings.

@paul1956
Copy link
Contributor Author

I created the file VisualBasicRefactoring-Settings.json under the project and added it to the Project File using

      VBSettings.Instance.WriteJsonFile(NewFileName)
      iProject.ProjectItems.AddFromFile(NewFileName)

What I get is the file added to the project under "
" as shown below

  <ItemGroup>
    <None Include="VBCodeRefactoring-Settings.json" />
  </ItemGroup>

But it should look like below, what API do I use to say it is an AdditionalFIle Vs. None?

<ItemGroup>
  <AdditionalFiles Include="VBCodeRefactoring-Settings.json" />
</ItemGroup>

@sharwell
Copy link
Member

...what API do I use to say it is an AdditionalFIle Vs. None?

@paul1956 If you are working specifically with a code fix (Roslyn APIs), you can't. We filed a bug for this in #4655, and eventually added related issue #5104. Other related issues include #4425 and #5096.

We are working around this in StyleCop Analyzers by hooking into the MSBuild process to treat None item types with a specific file name as an AdditionalFiles item (DotNetAnalyzers/StyleCopAnalyzers#1581).

To solve the separate issue of avoiding the use of individual configuration files in each project, we are using linked files (DotNetAnalyzers/StyleCopAnalyzers@59ec260 shows one configuration file shared across three projects). This process is not currently automated; if you use the code fix which creates a stylecop.json file it will create one file for each project. However you can always correct this by hand, which is hopefully a one-time operation.

@paul1956
Copy link
Contributor Author

@sharwell it is not obvious how to specify a Linked File in the Project file. Right now I just create a copy in every project with the solution except the VXIS, and when it is edited I edit every copy. In the example below where are you storing one common file, how does the feature work (what is the syntax)? Is this what requires manually editing of project file?

  <ItemGroup>
    <AdditionalFiles Include="..\stylecop.json">
      <Link>stylecop.json</Link>
    </AdditionalFiles>
  </ItemGroup>

@sharwell
Copy link
Member

@paul1956 Visual Studio has long had the ability to add a linked file to a C# project. From the Add → Existing Item... dialog, locate the file you wish to add and instead of clicking Add, click the dropdown arrow next to Add and select Add as Link. However, adding the file as a link is not sufficient because you'd end up with this:

<None Include="..\stylecop.json">
  <Link>stylecop.json</Link>
</None>

You still need to edit the project file by hand to replace None with AdditionalFiles (in this case twice).

@paul1956
Copy link
Contributor Author

VB just got that feature in VS2015 but I really want to avoid hand editing. How does the line know where to find the file? Is there any API to add a linked file? In my VSIX where I create the file(s) I would want to create the 2nd through nth as linked if possible. If not I see more feature requests.

From the solution object can add AdditionalFiles without hand editing but I don't see how to make then linked.

@mavasani
Copy link
Contributor

mavasani commented Nov 3, 2015

So, I feel this has turned into a very long discussion, and we have moved away from the original issue raised by Paul: what is supported mode for threading analyzer options from the user down to the analyzer?

  1. Per-project options: You can create AdditionalFile with the options and add these files to the project, and the analyzer can access them through AnalyzerOptions. Add support for document specific analyzer options #3798 tracks enabling threading such options from the project file to the analyzer.
  2. Workspace wide options: As @tmeschter explained earlier, analyzer has no concept of workspace and is unaware of the host that it is being executed within. What you probably want is Add public support for third party Document and Project diagnostic sources #3797 to be implemented so that you can write a VS-only diagnostic source, that knows that it is being executed within a workspace and hence has access to Workspace options.

Kindly re-open this issue if you feel you want some more info or feel the cited work items won't suffice your needs.

@mavasani mavasani closed this as completed Nov 3, 2015
@paul1956
Copy link
Contributor Author

paul1956 commented Nov 4, 2015

If all 5 features of #3798 are implemented then I think you will have provided me what I am looking for but the key are requirements 3 and 5. Both address the same issue, I don't want to continually parse the AdditionalFile to recreate a key/value pair especially when they "almost" never change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants