Skip to content

Comments

Add MA0182: Detect unused internal classes with correct handling of generic type arguments and typeof references#956

Merged
meziantou merged 20 commits intomainfrom
copilot/fork-fix-ca1812-issues
Jan 9, 2026
Merged

Add MA0182: Detect unused internal classes with correct handling of generic type arguments and typeof references#956
meziantou merged 20 commits intomainfrom
copilot/fork-fix-ca1812-issues

Conversation

Copy link
Contributor

Copilot AI commented Jan 8, 2026 •
edited
Loading

Implementation Plan: Fix CA1812 False Positives

  • Understand the issue and existing CA1812 problems
  • Explore repository structure and build system
  • Create new rule identifier MA0182 for "Avoid uninstantiated internal classes"
  • Implement analyzer to detect unused internal classes with proper handling for:
    • Generic type arguments (e.g., JsonSerializer.Deserialize<InternalClass>())
    • typeof() references in attributes
    • Classes used via reflection or serialization
    • Member access on constructed generic types (e.g., Sample<InternalClass>.Empty)
    • Consider InternalsVisibleTo attribute (not implemented as it's complex and optional)
  • Create comprehensive tests covering:
    • False positive scenarios from CA1812 issues
    • True positive scenarios (actually unused classes)
    • Edge cases (abstract classes, interfaces, etc.)
    • Generic type with static member access
    • XmlSerializer usage
    • Newtonsoft.Json serialization
    • YamlDotNet serialization
  • Build and test the implementation
  • Generate documentation
  • Address code review feedback
  • Address PR review comments
  • Update comparison-with-other-analyzers.md
  • Fix CI compilation error
  • Change severity to Info (suggestion)
  • Remove CA1812 mentions from rule documentation
  • Fix test compilation errors
  • Final validation with CodeQL
Original prompt

This section details on the original issue you should resolve

<issue_title>Fork and fix CA1812?</issue_title>
<issue_description>Is your feature request related to a problem? Please describe.
There are several known issues in CA1812:
dotnet/roslyn-analyzers#6561 (reported by ... :-))
dotnet/roslyn-analyzers#7787

I highly doubt MS is ever going to fix these, and I'm not sure they accept contributions.

Describe the solution you'd like
Introduce a similar rule in Meziantou that fixes both issues. That rule should probably also not report anything if the project exposes its internals, but still allow for an override. Personally, I only ever expose internals for unit test purposes and any class that is only ever used in a unit test can of course be removed.

Alternate solution would be a suppressor that "mutes" false positives for CA1812.

Ideally there would also be a rule that reports a diagnostic for unused internal (and even protected) members.
</issue_description>

Comments on the Issue (you are @copilot in this section)

💡 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.

Copilot AI and others added 2 commits January 8, 2026 18:43
@meziantou
Add MA0182: Avoid uninstantiated internal classes analyzer
6a6479f 
Implements a new analyzer that detects unused internal classes,
fixing known CA1812 false positives:
- Correctly handles classes used as generic type arguments
- Correctly handles classes referenced via typeof() in attributes

The analyzer:
- Tracks all internal classes in the compilation
- Monitors their usage via object creation, generic type arguments, typeof operations, and array creation
- Reports diagnostics for truly unused internal classes at compilation end

Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
@meziantou
Fix code review feedback: Improve AddUsedType to prevent infinite rec…
5ac9698 
…ursion

- Remove unused Compilation parameter from AnalyzerContext constructor
- Prevent infinite recursion in AddUsedType by checking if type was already added before processing
- Early return when type has already been processed

Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
Copilot AI changed the title [WIP] Introduce new rule to address CA1812 issues Add MA0182: Detect unused internal classes with correct handling of generic type arguments and typeof references Jan 8, 2026
Copilot AI requested a review from meziantou January 8, 2026 18:47
@meziantou
Add support for tracking internal classes used as type arguments in m…
ee8720f 
…ember references

- Register handlers for PropertyReference, FieldReference, MethodReference, and EventReference operations
- Track type arguments in the containing type when accessing members (e.g., Sample<InternalClass>.Empty)
- Add test case for generic type with static member access using internal class as type argument

Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
Copilot AI requested a review from meziantou January 8, 2026 20:39
@meziantou
Add tests for XML, Newtonsoft.Json, and YamlDotNet serialization
913631b 
- Add test for XmlSerializer with internal class passed via typeof()
- Add test for Newtonsoft.Json JsonConvert.DeserializeObject<T>() with internal class
- Add test for YamlDotNet Deserialize<T>() with internal class
- All tests verify that internal classes used by serializers are not incorrectly flagged as unused

Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
@meziantou
Copy link
Owner

meziantou commented Jan 8, 2026

@copilot Can you update docs/comparison-with-other-analyzers.md with the new rule and CA1812

@meziantou
Copy link
Owner

meziantou commented Jan 8, 2026

@copilot I get a compilation error on CI

Error: /home/runner/work/Meziantou.Analyzer/Meziantou.Analyzer/src/Meziantou.Analyzer/Rules/AvoidUninstantiatedInternalClassesAnalyzer.cs(21,45): error CS0117: 'WellKnownDiagnosticTags' does not contain a definition for 'CompilationEnd' [/home/runner/work/Meziantou.Analyzer/Meziantou.Analyzer/src/Meziantou.Analyzer/Meziantou.Analyzer.csproj::TargetFramework=net10.0]

meziantou
meziantou reviewed Jan 8, 2026
View reviewed changes
docs/Rules/MA0182.md Outdated
@@ -0,0 +1,53 @@
# MA0182 - Avoid uninstantiated internal classes

This analyzer detects internal classes that are never instantiated or referenced. This is similar to CA1812 but fixes several known issues:
Copy link
Owner

@meziantou meziantou Jan 8, 2026

Choose a reason for hiding this comment

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

@copilot do not mention ca1812 in the rule documentation.

Copy link
Contributor Author

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Removed all CA1812 mentions from the rule documentation. Commit: e5e6e83

@meziantou
Update comparison-with-other-analyzers.md to include MA0182 and CA1812
48ea4fa 
Add MA0182 to the "Similar rules" section showing that it correctly handles internal classes used as generic type arguments and in typeof() expressions, addressing CA1812's known false positives.

Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 8, 2026

@copilot Can you update docs/comparison-with-other-analyzers.md with the new rule and CA1812

Updated docs/comparison-with-other-analyzers.md to include MA0182 in the "Similar rules" section, showing its relationship to CA1812 and explaining that MA correctly handles internal classes used as generic type arguments and in typeof() expressions. Commit: 48ea4fa

meziantou
meziantou reviewed Jan 8, 2026
View reviewed changes
src/Meziantou.Analyzer/Rules/AvoidUninstantiatedInternalClassesAnalyzer.cs Outdated
title: "Avoid uninstantiated internal classes",
messageFormat: "Internal class '{0}' is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it 'static'.",
RuleCategories.Design,
DiagnosticSeverity.Warning,
Copy link
Owner

@meziantou meziantou Jan 8, 2026

Choose a reason for hiding this comment

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

@copilot Make it a suggestion by default

Copy link
Contributor Author

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Changed severity from Warning to Info (suggestion). Commit: e5e6e83

Copilot AI requested a review from meziantou January 8, 2026 21:23
@meziantou
Fix compilation error and update rule severity to Info
e5e6e83 
- Remove WellKnownDiagnosticTags.CompilationEnd (doesn't exist) and use string array ["CompilationEnd"] instead
- Change DiagnosticSeverity from Warning to Info (suggestion) as requested
- Remove CA1812 mentions from rule documentation
- Regenerate documentation files to reflect severity change

Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 8, 2026

@copilot I get a compilation error on CI

Error: /home/runner/work/Meziantou.Analyzer/Meziantou.Analyzer/src/Meziantou.Analyzer/Rules/AvoidUninstantiatedInternalClassesAnalyzer.cs(21,45): error CS01...

Fixed the compilation error by replacing WellKnownDiagnosticTags.CompilationEnd with a string array ["CompilationEnd"]. The constant doesn't exist in the Roslyn version being used. Commit: e5e6e83

This was referenced Feb 14, 2026
Closed
Open
Closed
Closed
Open
Open
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Open
Closed
Closed
Closed
Closed
Open
Closed
Closed
Open
Closed
This was referenced Feb 21, 2026
Open
Open
Open
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@meziantou meziantou meziantou left review comments

Assignees

@meziantou meziantou

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fork and fix CA1812?

2 participants

@meziantou