Skip to content

Conversation

@SENya1990
Copy link
Collaborator

Changes Overview

  • fixed the check of the delegate parameter name + changed the parameter name passed to the code fix for PX1102
  • added unit tests for PX1102 for the overridden property

Copy link
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 fixes a bug where PX1102 incorrectly flagged property overrides with false alerts for invalid delegate parameter names. The fix corrects the delegate parameter name validation logic and updates the parameter name passed to the code fix provider.

Changes:

  • Modified delegate parameter name validation to handle property accessors correctly
  • Added new helper method GetProperNameOfDelegateParameter to extract appropriate base names for properties
  • Added comprehensive unit tests for property override scenarios with incorrect delegate parameter names

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
PXOverrideAnalyzer.cs Fixed delegate parameter name validation logic to handle property accessors and extract proper property names
PXOverrideWithIncorrectDelegateParameterNameTests.cs Added three new test methods to verify property override validation and code fix behavior
PXOverrideOfPropertyFromBasePXGraphWithIncorrectDelegateParameterName.cs Added test source file with incorrect delegate parameter name for property override
PXOverrideOfPropertyFromBasePXGraphWithIncorrectDelegateParameterName_Expected.cs Added expected test output after code fix is applied

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +192 to +193
string baseMethodName = baseMethod.AssociatedSymbol?.Name.NullIfWhiteSpace() ?? baseMethod.Name;
return baseMethodName;
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The variable baseMethodName is assigned and immediately returned. Consider returning the expression directly: return baseMethod.AssociatedSymbol?.Name.NullIfWhiteSpace() ?? baseMethod.Name;

Suggested change
string baseMethodName = baseMethod.AssociatedSymbol?.Name.NullIfWhiteSpace() ?? baseMethod.Name;
return baseMethodName;
return baseMethod.AssociatedSymbol?.Name.NullIfWhiteSpace() ?? baseMethod.Name;

Copilot uses AI. Check for mistakes.
MethodKind.PropertySet or
MethodKind.EventAdd or
MethodKind.EventRemove or
MethodKind.EventRaise => baseMethod.AssociatedSymbol,
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Inconsistent use of tabs and spaces in the alignment. Line 280 uses tabs while lines 276-279 use spaces. This creates irregular spacing that may appear differently across editors with different tab width settings.

Suggested change
MethodKind.EventRaise => baseMethod.AssociatedSymbol,
MethodKind.EventRaise => baseMethod.AssociatedSymbol,

Copilot uses AI. Check for mistakes.
Base automatically changed from bugfix/ATR-939-dev-PX1098-code-fix-handle-out-and-ref-parameters to dev January 20, 2026 13:53
@SENya1990 SENya1990 merged commit 917b05c into dev Jan 20, 2026
@SENya1990 SENya1990 deleted the bugfix/ATR-934-dev-PX1102-false-alert-for-property-override branch January 20, 2026 13:56
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.

3 participants