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

Remove parameter which was always passed the same value #73198

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 23, 2024 19:01
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 23, 2024
@@ -24,7 +24,7 @@ internal partial class DiagnosticIncrementalAnalyzer
/// Return all diagnostics that belong to given project for the given StateSets (analyzers) either from cache or by calculating them
/// </summary>
private async Task<ProjectAnalysisData> GetProjectAnalysisDataAsync(
CompilationWithAnalyzers? compilationWithAnalyzers, Project project, IdeAnalyzerOptions ideOptions, ImmutableArray<StateSet> stateSets, bool forceAnalyzerRun, CancellationToken cancellationToken)
Copy link
Member Author

Choose a reason for hiding this comment

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

this function was always passed 'true' for 'forceAnalyzerRun'. I removed the parameter, and collapsed all the control flow accordingly.

fullAnalysisEnabled = true;
compilerFullAnalysisEnabled = true;
analyzersFullAnalysisEnabled = true;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

since forceAnalyzerRun, these 3 values were always true.

analyzersFullAnalysisEnabled = true;
}

if (!fullAnalysisEnabled)
Copy link
Member Author

Choose a reason for hiding this comment

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

since fullAnalysisEnabled was always true (due to forceAnalyzerRun being true), this block was never run.


// Reduce the state sets to analyze based on individual full solution analysis values
// for compiler diagnostics and analyzers.
if (!compilerFullAnalysisEnabled)
Copy link
Member Author

Choose a reason for hiding this comment

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

same for these two conditions.

@@ -179,8 +122,8 @@ private static bool CompilationHasOpenFileOnlyAnalyzers(CompilationWithAnalyzers
if (compilationWithAnalyzers?.Analyzers.Length > 0)
{
// calculate regular diagnostic analyzers diagnostics
var resultMap = await _diagnosticAnalyzerRunner.AnalyzeProjectAsync(project, compilationWithAnalyzers,
forcedAnalysis, logPerformanceInfo: false, getTelemetryInfo: true, cancellationToken).ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

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

that value eventually flowed into this AnalyzeProjectAsync call. This is the only place in teh product (not tests) where this method is called. so the parameter got removed here as well.

bool logPerformanceInfo,
bool getTelemetryInfo,
CancellationToken cancellationToken)
=> AnalyzeAsync(documentAnalysisScope: null, project, compilationWithAnalyzers,
isExplicit: false, forceExecuteAllAnalyzers, logPerformanceInfo, getTelemetryInfo, cancellationToken);
isExplicit: false, forceExecuteAllAnalyzers: true, logPerformanceInfo, getTelemetryInfo, cancellationToken);
Copy link
Member Author

Choose a reason for hiding this comment

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

the parameter was passed in here, as AnalyzeAsync is called from AnalyzeDocumentAsync above, which passesin in 'false' for this value.

@@ -192,11 +192,7 @@ void Method()
analyzerReference.GetAnalyzers(project.Language).Where(a => a.GetType() == analyzerType).ToImmutableArray(),
new WorkspaceAnalyzerOptions(project.AnalyzerOptions, ideAnalyzerOptions));

// no result for open file only analyzer unless forced
var result = await runner.AnalyzeProjectAsync(project, compilationWithAnalyzers, forceExecuteAllAnalyzers: false, logPerformanceInfo: false, getTelemetryInfo: false, cancellationToken: CancellationToken.None);
Assert.Empty(result.AnalysisResult);
Copy link
Member Author

Choose a reason for hiding this comment

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

this test validated behavior that the product never ran into.

@CyrusNajmabadi CyrusNajmabadi merged commit 2c1fc68 into dotnet:main Apr 23, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Apr 23, 2024
@CyrusNajmabadi CyrusNajmabadi deleted the removeParameter branch April 23, 2024 20:40
@dibarbet dibarbet modified the milestones: Next, 17.11 P1 Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants