-
Notifications
You must be signed in to change notification settings - Fork 9
Bugfix/atr 934 dev px1102 false alert for property override #657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfix/atr 934 dev px1102 false alert for property override #657
Conversation
…parameter name passed to the code fix for PX1102
There was a problem hiding this 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
GetProperNameOfDelegateParameterto 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.
| string baseMethodName = baseMethod.AssociatedSymbol?.Name.NullIfWhiteSpace() ?? baseMethod.Name; | ||
| return baseMethodName; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
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;
| string baseMethodName = baseMethod.AssociatedSymbol?.Name.NullIfWhiteSpace() ?? baseMethod.Name; | |
| return baseMethodName; | |
| return baseMethod.AssociatedSymbol?.Name.NullIfWhiteSpace() ?? baseMethod.Name; |
| MethodKind.PropertySet or | ||
| MethodKind.EventAdd or | ||
| MethodKind.EventRemove or | ||
| MethodKind.EventRaise => baseMethod.AssociatedSymbol, |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
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.
| MethodKind.EventRaise => baseMethod.AssociatedSymbol, | |
| MethodKind.EventRaise => baseMethod.AssociatedSymbol, |
Changes Overview