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

Fix TypeInitializationException in SymbolStartAnalysisContextWrapper #8991

Conversation

martin-strecker-sonarsource
Copy link
Contributor

Fixes AD0001
See https://cirrus-ci.com/task/5657719948967936?logs=build#L1542

CSC : error AD0001: Analyzer 
'SonarAnalyzer.Rules.CSharp.RouteTemplateShouldNotStartWithSlash' threw an exception of type 
'System.TypeInitializationException' with message 'The type initializer for 
'SonarAnalyzer.ShimLayer.AnalysisContext.SymbolStartAnalysisContextWrapper' 
threw an exception.'.

I had a UT for the fix, but it broke all the other tests because the type initializer is called only once per assembly load. I tried with AppDomains, but that didn't work. The only solution would be to isolate the test in a dedicated UT assembly, which does seem to be overkill.

@martin-strecker-sonarsource martin-strecker-sonarsource added Type: Bug Exceptions and blocking issues during analysis. Area: VB.NET VB.NET rules related issues. Area: C# C# rules related issues. labels Mar 28, 2024
@github-actions github-actions bot added this to In progress in Best Kanban Mar 28, 2024
if (symbolStartAnalysisContextType == null)
{
return static _ => default;
}
var receiverParameter = Parameter(typeof(object));
return Lambda<Func<object, TProperty>>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Lambda() invocation throws an ArgumentException if symbolStartAnalysisContextType is null. That one caused the TypeInitializationException

Copy link

sonarcloud bot commented Mar 28, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Mar 28, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Contributor

@mary-georgiou-sonarsource mary-georgiou-sonarsource left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot moved this from In progress to Review approved in Best Kanban Mar 28, 2024
Copy link
Contributor

@antonioaversa antonioaversa left a comment

Choose a reason for hiding this comment

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

LGTM

I had a UT for the fix, but it broke all the other tests because the type initializer is called only once per assembly load.

Out of curiosity, could you share the UT. I would like to see how to reproduce the symbolStartAnalysisContextType being null.

@mary-georgiou-sonarsource
Copy link
Contributor

@martin-strecker-sonarsource @antonioaversa is there a reason this is draft? Can we merge it?

@martin-strecker-sonarsource
Copy link
Contributor Author

Out of curiosity, could you share the UT. I would like to see how to reproduce the symbolStartAnalysisContextType being null.

Ugly hack, which looked about like this (The SymbolStartAnalysisContextTypeName property was actually in another class because the TypeInitializer runs before the assignment is executed):

// In the UT:
SymbolStartAnalysisContextWrapper.SymbolStartAnalysisContextTypeName = "SomeNonExitingType";

// In the Wrapper
public static string SymbolStartAnalysisContextTypeName = "Microsoft.CodeAnalysis.Diagnostics.SymbolStartAnalysisContext";
// And in the Initializer:
var symbolStartAnalysisContextType = typeof(CompilationStartAnalysisContext).Assembly.GetType(SymbolStartAnalysisContextTypeName);

@martin-strecker-sonarsource martin-strecker-sonarsource marked this pull request as ready for review March 28, 2024 14:19
@martin-strecker-sonarsource martin-strecker-sonarsource merged commit 6f9fee8 into master Mar 28, 2024
36 checks passed
@martin-strecker-sonarsource martin-strecker-sonarsource deleted the Martin/AD001_TypeInitilizerSymbolStartAnalysisContext branch March 28, 2024 14:19
Best Kanban automation moved this from Review approved to Validate Peach Mar 28, 2024
@antonioaversa
Copy link
Contributor

antonioaversa commented Mar 28, 2024

Out of curiosity, could you share the UT. I would like to see how to reproduce the symbolStartAnalysisContextType being null.

Ugly hack, which looked about like this (The SymbolStartAnalysisContextTypeName property was actually in another class because the TypeInitializer runs before the assignment is executed):

// In the UT:
SymbolStartAnalysisContextWrapper.SymbolStartAnalysisContextTypeName = "SomeNonExitingType";

// In the Wrapper
public static string SymbolStartAnalysisContextTypeName = "Microsoft.CodeAnalysis.Diagnostics.SymbolStartAnalysisContext";
// And in the Initializer:
var symbolStartAnalysisContextType = typeof(CompilationStartAnalysisContext).Assembly.GetType(SymbolStartAnalysisContextTypeName);

I see, ugly indeed, I must say 😆
Thanks for clarifying!

@martin-strecker-sonarsource
Copy link
Contributor Author

Peach validation: AD0001 is gone:
https://cirrus-ci.com/task/5582737403281408

@martin-strecker-sonarsource martin-strecker-sonarsource moved this from Validate Peach to Done in Best Kanban Apr 2, 2024
@mary-georgiou-sonarsource mary-georgiou-sonarsource added this to the 9.23.1 milestone Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Area: VB.NET VB.NET rules related issues. Type: Bug Exceptions and blocking issues during analysis.
Projects
Best Kanban
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants