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

Ensure that we respect source suppressions for IDE analyzer diagnosti… #11343

Merged
merged 1 commit into from
May 17, 2016

Conversation

mavasani
Copy link
Contributor

…cs (analyzer dependency errors, reading ruleset errors, etc.)

Fixes #8877

@mavasani
Copy link
Contributor Author

@heejaechang @dotnet/roslyn-analysis can you please review?

@mavasani
Copy link
Contributor Author

Ping

_updateSource.UpdateDiagnosticsForProject(projectId, Tuple.Create(s_analyzerChangedErrorId, analyzerPath), SpecializedCollections.SingletonEnumerable(data));
var messageArguments = new string[] { analyzerPath };
var diagnostic = await DiagnosticData.CreateAsync(_analyzerChangedRule, messageArguments, projectId, _workspace, CancellationToken.None).ConfigureAwait(false);
_updateSource.UpdateDiagnosticsForProject(projectId, Tuple.Create(s_analyzerChangedErrorId, analyzerPath), SpecializedCollections.SingletonEnumerable(diagnostic));
Copy link
Contributor

Choose a reason for hiding this comment

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

is it okay to be async now? and order of event become changed? for example, it raised event (but not registered to update source yet) but before that happens, solution is closed and then it is registered to update source.

@heejaechang
Copy link
Contributor

left some comment, I think isSuppressed argument is a bit weird since diagnostic and issuppressed can be 2 different value given by users.

…cs (analyzer dependency errors, reading ruleset errors, etc.)

Fixes dotnet#8877
@mavasani
Copy link
Contributor Author

@heejaechang Thanks. I have addressed your feedback and it should be much cleaner now.

@mavasani
Copy link
Contributor Author

@heejaechang Does the new changes look good?

{
// Get diagnostic with effective severity.
// Additionally, if the diagnostic was suppressed by a source suppression, effectiveDiagnostics will have a diagnostic with IsSuppressed = true.
var compilation = project.GetCompilationAsync(cancellationToken).WaitAndGetResult(cancellationToken);
Copy link
Contributor

Choose a reason for hiding this comment

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

will this be okay?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, but all the current callsites are sync. We can revisit in future.

@heejaechang
Copy link
Contributor

👍

@mavasani mavasani merged commit dcd141b into dotnet:master May 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants