-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Enforce CA1854 performance analyzer #22100
base: main
Are you sure you want to change the base?
Conversation
0a5e440
to
76118a9
Compare
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not compiling. Needs more changes in the ApplyPropertiesVisitor
class.
D:\a\_work\1\s\src\Controls\src\Xaml\ApplyPropertiesVisitor.cs(362,61): error CS0165: Use of unassigned local variable 'value' [D:\a\_work\1\s\src\Controls\src\Xaml\Controls.Xaml.csproj::TargetFramework=net8.0-windows10.0.20348.0]
21 Error(s)
Fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, looks like there are some legitimate failures in the build now that the analyzer is an error:
/Users/builder/azdo/_work/1/s/src/Core/src/Platform/Tizen/WindowExtensions.cs(24,8): error CA1854: Prefer a 'TryGetValue' call over a Dictionary indexer access guarded by a 'ContainsKey' check to avoid double lookup (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1854) [/Users/builder/azdo/_work/1/s/src/Core/src/Core.csproj::TargetFramework=net8.0-tizen]
/Users/builder/azdo/_work/1/s/src/Core/src/Platform/Tizen/WindowExtensions.cs(65,8): error CA1854: Prefer a 'TryGetValue' call over a Dictionary indexer access guarded by a 'ContainsKey' check to avoid double lookup (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1854) [/Users/builder/azdo/_work/1/s/src/Core/src/Core.csproj::TargetFramework=net8.0-tizen]
/Users/builder/azdo/_work/1/s/src/Core/src/Platform/Tizen/WindowExtensions.cs(106,8): error CA1854: Prefer a 'TryGetValue' call over a Dictionary indexer access guarded by a 'ContainsKey' check to avoid double lookup (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1854) [/Users/builder/azdo/_work/1/s/src/Core/src/Core.csproj::TargetFramework=net8.0-tizen]
/Users/builder/azdo/_work/1/s/src/Core/src/Platform/Tizen/WindowExtensions.cs(112,8): error CA1854: Prefer a 'TryGetValue' call over a Dictionary indexer access guarded by a 'ContainsKey' check to avoid double lookup (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1854) [/Users/builder/azdo/_work/1/s/src/Core/src/Core.csproj::TargetFramework=net8.0-tizen]
/Users/builder/azdo/_work/1/s/src/Core/src/Platform/Tizen/StackNavigationManager.cs(196,8): error CA1854: Prefer a 'TryGetValue' call over a Dictionary indexer access guarded by a 'ContainsKey' check to avoid double lookup (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1854) [/Users/builder/azdo/_work/1/s/src/Core/src/Core.csproj::TargetFramework=net8.0-tizen]
Looks like I should add the Tizen workload locally. Fixed those builds. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Some more errors 😢
|
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Rerun failed tests? |
Yup trying now. |
This test seems to be failing consistently:
So maybe there's some problem in the change itself? |
Given that this PR does change code to do with finding factory methods, and that this particular test has failed multiple times in the PR, I think it warrants an investigation of the code in the PR. Maybe there's some subtle change in terms of how the code now locates matches for constructors somehow? |
This is hopefully a straightforward change. Similar to #22092
See: CA1854