Skip to content

BaseDiagnosticIncrementalAnalyzer doc comments - #747

Merged
msJohnHamby merged 8 commits into
dotnet:masterfrom
msJohnHamby:BaseAnalyzerDocComments
Feb 21, 2015
Merged

BaseDiagnosticIncrementalAnalyzer doc comments#747
msJohnHamby merged 8 commits into
dotnet:masterfrom
msJohnHamby:BaseAnalyzerDocComments

Conversation

@msJohnHamby

Copy link
Copy Markdown
Contributor

In preparation for creating a new diagnostic incremental analyzer, I've been attempting to derive the semantics of the contract it must obey. This change set includes partial doc comments for the BaseDiagnosticIncrementalAnalyzer class, intended to capture significant aspects of my inferences. Please correct any misunderstandings or significant omissions.

I'm especially trying to get the attention of: @heejaechang @mavansani @srivatsn

Add partial doc comments to BaseDiagnosticIncrementalAnalyzer sufficient
for others to validate my understanding.
@msJohnHamby

Copy link
Copy Markdown
Contributor Author

I meant @mavasani

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

here solution is given as a way for implementer to extract required information for their cache (what information is needed will vary based on their cache). such as document, project, versions, workspace and etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, so "ignored" is obviously incorrect. In digging through the logic of the current implementation, it appears as if there is a rule in place of:
-- If a document is supplied, return diagnostics for that document.
-- Otherwise, if a project is supplied, return diagnostics for that project.
-- Otherwise, return diagnostics for the solution.
Is this correct? Assuming so, I'll amend the doc comments to be closer to correct.

@heejaechang

Copy link
Copy Markdown
Contributor

👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you remove the < param > nodes if there is nothing interesting to say about them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

VS produces a warning, which turns into an error, if a parameter does not have a corresponding element.

I'll try to put some meaningful text in all the elements now--I wanted to draw attention to the points where my inferences might have been incorrect.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If there are no param tags at all then VS won’t produce the error. It’s only when some of the params have tags that happens.

From: JohnHamby [mailto:notifications@github.com]
Sent: Friday, February 20, 2015 5:23 PM
To: dotnet/roslyn
Cc: Srivatsn Narayanan
Subject: Re: [roslyn] BaseDiagnosticIncrementalAnalyzer doc comments (#747)

In src/Features/Core/Diagnostics/BaseDiagnosticIncrementalAnalyzer.cshttps://github.com//pull/747#discussion_r25116906:

@@ -19,24 +19,145 @@ protected BaseDiagnosticIncrementalAnalyzer(Workspace workspace)

     }



     #region IIncrementalAnalyzer
  •    /// <summary>
    
  •    /// Analyze a single document such that local diagnostics for that document become available,
    
  •    /// prioritizing analyzing this document over analyzing the rest of the project.
    
  •    /// Calls <see cref="DiagnosticAnalyzerService.RaiseDiagnosticsUpdated(object, DiagnosticsUpdatedArgs)"/> for each
    
  •    /// unique group of diagnostics, where a group is identified by analysis classification (syntax/semantics), document, and analyzer.
    
  •    /// </summary>
    
  •    /// <param name="document"></param>
    

VS produces a warning, which turns into an error, if a parameter does not have a corresponding element.

I'll try to put some meaningful text in all the elements now--I wanted to draw attention to the points where my inferences might have been incorrect.


Reply to this email directly or view it on GitHubhttps://github.com//pull/747/files#r25116906.

@srivatsn

Copy link
Copy Markdown
Contributor

👍

msJohnHamby added a commit that referenced this pull request Feb 21, 2015
BaseDiagnosticIncrementalAnalyzer doc comments

Added doc comments to the BaseDiagnosticIncrementalAnalyzer class sufficient to guide a new implementation of an incremental analyzer engine.

(Also, this makes for a relatively safe set of changes for my first pull request merge.)
@msJohnHamby
msJohnHamby merged commit 5738b06 into dotnet:master Feb 21, 2015
dibarbet pushed a commit that referenced this pull request Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants