E543 [Mvc] Avoid exception in route analyzer by martincostello · Pull Request #63033 · dotnet/aspnetcore · GitHub
[go: up one dir, main page]

Skip to content

Conversation

martincostello
Copy link
Member

Avoid exception in route analyzer

Avoid throwing if IMethodSymbol.OverriddenMethod is null.

Description

Avoid throwing if IMethodSymbol.OverriddenMethod is null if IMethodSymbol.IsOverride is true.

I would have written a test, but there's no repo in the issue and I have no idea what would make this eventuality occur.

Fixes #59736

@Copilot Copilot AI review requested due to automatic review settings August 1, 2025 10:41
@github-actions github-actions bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Aug 1, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 1, 2025
Copy link
Contributor
@Copilot 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 fixes a potential exception in the MVC route analyzer by handling cases where IMethodSymbol.OverriddenMethod is unexpectedly null despite IMethodSymbol.IsOverride being true. Instead of throwing an exception, the code now gracefully returns null and treats such methods as non-controller actions.

Key Changes

  • Replace exception throwing with graceful null handling in GetDeclaringType method
  • Update return type annotation to reflect nullable return value
  • Add null check for declaring type before accessing properties

Copy link
Member
@captainsafia captainsafia 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 won't gate on lack of tests in this case since we're notoriously lacking on test coverage for MVC analyzers and this particular scenario will be hard to change. The delta is small and defensive enough that it's not too concerning.

Let's hope I don't eat my words! 😄

@captainsafia captainsafia merged commit 5d0e0d0 into dotnet:main Aug 1, 2025
31 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-rc1 milestone Aug 1, 2025
@martincostello martincostello deleted the gh-59736 branch August 2, 2025 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates community-contribution Indicates that the PR has been added by a community member
Projects
4B10
None yet
Development

Successfully merging this pull request may close these issues.

Feature Semantic classification is currently unavailable due to an internal error
2 participants
0