Implement PH2163: Avoid NoWarn project settings for diagnostic suppression by Copilot · Pull Request #999 · philips-software/roslyn-analyzers

Copilot · 2025-09-09T16:25:14Z

This PR implements a new analyzer PH2163 that detects when NoWarn project settings are used to suppress diagnostic warnings instead of the preferred .editorconfig approach.

Problem

Developers were using <NoWarn> elements in project files to suppress diagnostic warnings:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <NoWarn>$(NoWarn);CS8002;PH2071</NoWarn>
  </PropertyGroup>
</Project>

This approach is discouraged because it lacks the granular control, team consistency, and IDE integration benefits of .editorconfig files.

Solution

The new AvoidNoWarnAnalyzerSuppressionAnalyzer detects any NoWarn usage in project settings and suggests using .editorconfig instead:

Detection: Scans project files (.csproj, .vbproj) found via source directory traversal
Simplified Logic: Detects any <NoWarn> element usage for comprehensive coverage
Performance: Lightweight string-based detection without XML parsing overhead
Error Handling: No try-catch blocks for dogfood build compliance
Diagnostic Message: Provides clear guidance with .editorconfig syntax

Example Output

Before (triggers PH2163):

<NoWarn>$(NoWarn);CS8002;PH2071</NoWarn>

After (recommended approach):

# .editorconfig
[*.cs]
dotnet_diagnostic.CS8002.severity = none
dotnet_diagnostic.PH2071.severity = none

Technical Details

Base Class: SolutionAnalyzer (enabled by default)
File Discovery: Uses proven pattern from LicenseAnalyzer for finding project files
String Detection: Simple case-insensitive string matching for performance
Dogfood Compliant: No empty catch blocks or analyzer violations

Benefits of .editorconfig over NoWarn

Granular control: Configure different severities (none, suggestion, warning, error)
File-specific configuration: Apply rules to specific file patterns
Team consistency: Shared configuration across different IDEs and tools
Version control friendly: Easy to track and review analyzer configuration changes
IDE integration: Better support in Visual Studio and other editors

Fixes #998.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>

Brian Collamore (bcollamore) · 2025-09-10T16:02:24Z

Documentation/Diagnostics/PH2163.md

+| Diagnostic ID | PH2163 |
+| Category  | [Maintainability](../Maintainability.md) |
+| Analyzer | [AvoidNoWarnAnalyzerSuppressionAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MaintainabilityAnalyzers/Maintainability/AvoidNoWarnAnalyzerSuppressionAnalyzer.cs)
+| CodeFix  | No |


Let's enable by default

Enabled by default in commit 7453880. Updated both the analyzer constructor (isEnabled: true) and documentation.

Brian Collamore (bcollamore) · 2025-09-10T16:04:15Z

...eAnalysis.MaintainabilityAnalyzers/Maintainability/AvoidNoWarnAnalyzerSuppressionAnalyzer.cs

+
+				AnalyzeProjectFile(context, projectFilePath);
+			}
+			catch (Exception)


Remember dogfood builds, which will complain about an empty catch block. In this scenario, unless you know of a reason this could happen, I'm tempted to advise trying the removal of it, and seeing how it performs. (We don't generally wrap analyzers in blank try...catch blocks (but maybe we should??)

Removed the empty catch blocks in commit 7453880. The analyzer now operates without try-catch wrapping, which should be compatible with dogfood builds.

Brian Collamore (bcollamore) · 2025-09-10T16:06:02Z

...eAnalysis.MaintainabilityAnalyzers/Maintainability/AvoidNoWarnAnalyzerSuppressionAnalyzer.cs

+					}
+
+					// Find PH analyzer codes in NoWarn value
+					List<string> phCodes = ExtractAnalyzerCodes(noWarnValue);


I think it's reasonable to complain about any usage of NoWarn. For niche use cases (3rd party stuff, legacy stuff, etc., it would make sense to just disable this rule.)

If we go this route, we presumably could eliminate the overhead of XDocument as well. Just search for the existence of ?

Simplified to detect any NoWarn usage in commit 7453880. Removed XDocument overhead and regex complexity - now uses simple string-based detection for better performance. The analyzer will flag any <NoWarn> element usage, and users can disable the rule for legitimate cases.

Brian Collamore (bcollamore) · 2025-09-10T16:07:27Z

Copilot see above. (Also, could you, as a first step, call the custom MCP server's run_tests tool? It has been giving us trouble and we think we fixed it; I'd like to test it.)

…y to detect any NoWarn usage Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>

Copilot · 2025-09-10T16:20:24Z

Copilot see above. (Also, could you, as a first step, call the custom MCP server's run_tests tool? It has been giving us trouble and we think we fixed it; I'd like to test it.)

The MCP server's run_tests tool initially timed out after 45 seconds, but I was able to run tests successfully using the bash tool with a longer timeout. All 2079 tests are passing with the changes in commit 7453880.

Brian Collamore (bcollamore) · 2025-10-09T15:16:08Z

Copilot Let's do an experiment unrelated to this Issue. Please call run_tests directly through our roslyn_analyzers_dev MCP server. I have had issues where at times, the first time the MCP server tool is called, it times out. I want to see if that's the case.

Brian Collamore (bcollamore) · 2025-10-09T15:18:30Z

Copilot Let's do an experiment unrelated to this Issue. Please call run_tests directly through our roslyn_analyzers_dev MCP server. I have had issues where at times, the first time the MCP server tool is called, it times out. I want to see if that's the case.

Brian Collamore (bcollamore) · 2025-10-09T15:19:12Z

Copilot are you sleeping? (pls see comments above)

Copilot · 2025-10-09T15:27:00Z

Copilot are you sleeping? (pls see comments above)

Called run_tests - it timed out after 45 seconds as you suspected. The timeout needs to be increased to 2+ minutes for the full test suite.

Copilot Let's do an experiment unrelated to this Issue. Please call run_tests directly through our roslyn_analyzers_dev MCP server. I have had issues where at times, the first time the MCP server too...

The run_tests MCP server tool timed out after 45 seconds. According to the repository instructions, the full test suite takes ~48 seconds to run 1903 tests, so the current 45-second timeout is insufficient. The tool would need a timeout of at least 2+ minutes to reliably complete.

Brian Collamore (bcollamore) · 2025-10-13T14:03:26Z

Copilot Let's do an experiment unrelated to this Issue. Please call run_tests directly through our roslyn_analyzers_dev MCP server. I have had issues where at times, the first time the MCP server tool is called, it times out. I want to see if that's still the case, as we just merged in a supposed fix via an unrelated PR.

Copilot · 2025-10-13T14:06:05Z

Copilot Let's do an experiment unrelated to this Issue. Please call run_tests directly through our roslyn_analyzers_dev MCP server. I have had issues where at times, the first time the MCP server to...

The run_tests tool still times out after 45 seconds. The supposed fix doesn't appear to have resolved the timeout issue - the tool still needs a longer timeout (2+ minutes) to complete the full test suite.

Initial plan

69f7600

Copilot AI assigned Copilot and Brian Collamore (bcollamore) Sep 9, 2025

Copilot started work on behalf of Brian Collamore (bcollamore) September 9, 2025 16:25 View session

Copilot AI and others added 2 commits September 9, 2025 16:39

Implement PH2163: Avoid NoWarn project settings for analyzer suppression

4c72bd2

Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>

Fix formatting issues in PH2163 analyzer - final implementation complete

013c3be

Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] PH2163: Avoid <NoWarn>~~ Implement PH2163: Avoid NoWarn project settings for analyzer suppression Sep 9, 2025

Copilot AI requested a review from Brian Collamore (bcollamore) September 9, 2025 16:50

Copilot finished work on behalf of Brian Collamore (bcollamore) September 9, 2025 16:50

Brian Collamore (bcollamore) requested changes Sep 10, 2025

View reviewed changes

Copilot started work on behalf of Brian Collamore (bcollamore) September 10, 2025 16:07 View session

Address review feedback: enable by default, remove try-catch, simplif…

7453880

…y to detect any NoWarn usage Co-authored-by: bcollamore <57269455+bcollamore@users.noreply.github.com>

Copilot AI changed the title ~~Implement PH2163: Avoid NoWarn project settings for analyzer suppression~~ Implement PH2163: Avoid NoWarn project settings for diagnostic suppression Sep 10, 2025

Copilot AI requested a review from Brian Collamore (bcollamore) September 10, 2025 16:21

Copilot finished work on behalf of Brian Collamore (bcollamore) September 10, 2025 16:21

Copilot started work on behalf of Brian Collamore (bcollamore) October 9, 2025 15:24 View session

Copilot finished work on behalf of Brian Collamore (bcollamore) October 9, 2025 15:27

Copilot started work on behalf of Brian Collamore (bcollamore) October 13, 2025 14:03 View session

Copilot finished work on behalf of Brian Collamore (bcollamore) October 13, 2025 14:06

Conversation

Copilot AI commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Example Output

Technical Details

Benefits of .editorconfig over NoWarn

Uh oh!

Brian Collamore (bcollamore) Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Brian Collamore (bcollamore) Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Brian Collamore (bcollamore) Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Brian Collamore (bcollamore) commented Sep 10, 2025

Uh oh!

Copilot AI commented Sep 10, 2025

Uh oh!

Brian Collamore (bcollamore) commented Oct 9, 2025

Uh oh!

Brian Collamore (bcollamore) commented Oct 9, 2025

Uh oh!

Brian Collamore (bcollamore) commented Oct 9, 2025

Uh oh!

Copilot AI commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Brian Collamore (bcollamore) commented Oct 13, 2025

Uh oh!

Copilot AI commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Sep 9, 2025 •

edited

Loading

Copilot AI commented Oct 9, 2025 •

edited

Loading