Skip to content

feat: Also consider typeof(T) as a constant#882

Merged
mikkelbu merged 1 commit intomasterfrom
feat/also-consider-typeof-constant
Jun 9, 2025
Merged

feat: Also consider typeof(T) as a constant#882
mikkelbu merged 1 commit intomasterfrom
feat/also-consider-typeof-constant

Conversation

@mikkelbu
Copy link
Copy Markdown
Member

@mikkelbu mikkelbu commented Jun 8, 2025

Fixes #880

@mikkelbu mikkelbu added this to the Release 4.9 milestone Jun 8, 2025
@mikkelbu mikkelbu requested a review from Copilot June 8, 2025 20:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the constant‐value analyzer to recognize typeof(T) as a constant and adds a test for Assert.That using typeof.

  • Extend the early-return condition to skip reporting on typeof operations
  • Add a new unit test targeting Assert.That(typeof(string), …)

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/nunit.analyzers/ConstActualValueUsage/ConstActualValueUsageAnalyzer.cs Allow OperationKind.TypeOf through the constant-value check
src/nunit.analyzers.tests/ConstActualValueUsage/ConstActualValueUsageAnalyzerTests.cs Add test covering Assert.That when the actual argument is typeof
Comments suppressed due to low confidence (1)

src/nunit.analyzers.tests/ConstActualValueUsage/ConstActualValueUsageAnalyzerTests.cs:472

  • Add a complementary test for other assertion methods (e.g., Assert.AreEqual(typeof(X), ...)) to ensure the analyzer consistently flags typeof across all assertion patterns.
[Test]

@mikkelbu mikkelbu requested a review from manfred-brands June 8, 2025 21:04
Copy link
Copy Markdown
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

Thanks @mikkelbu.
No comments looks good.

Copy link
Copy Markdown
Member

@stevenaw stevenaw left a comment

Choose a reason for hiding this comment

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

LGTM as well, thanks for the prompt update @mikkelbu

@mikkelbu mikkelbu merged commit fa9b2ac into master Jun 9, 2025
7 checks passed
@mikkelbu mikkelbu deleted the feat/also-consider-typeof-constant branch June 9, 2025 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NUnit2007 could flag typeof() as a constant first parameter

4 participants