Add Roslyn analyzer for event sink helper consistency (NOE001-NOE003)#473
Add Roslyn analyzer for event sink helper consistency (NOE001-NOE003)#473
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47a859f19e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
I tried the analyzer in random class that derives from SinkHelper (RecordsetEvents_SinkHelper) but I found out it does not work here because of #pragma warning disable preprocessor directive preventing warnings for whole file.
There are more classes deriving from SinkHelper with the warnings disabled so the analyzer would not be able to catch the event name mismatch there also. Even changing the NOE001 to Error will not help, since those will be also hidden.
I would focus all the preprocessor directives hiding warnings in all the files contaning classes derived from SinkHelper to e.g. #pragma warning disable CS1591 since I assume that warning was a reason why it was used in first place. This way we will get shown valid warnings from new analyzer and can fix them.
We will keep the warning CS1591 (Missing XML comment for publicly visible type or member) disabled as many members are missing documentation comments
878ef99 to
55b26b3
Compare
Add Roslyn analyzer for event sink helper consistency (NOE001-NOE003)
Summary
EventValidateRaiseConsistencyAnalyzer) that detects common bugs in NetOffice event sink helpers where the event name passed toValidate()doesn't match the event name passed toRaiseCustomEvent()NetOffice.SinkHelperto avoid false positives on unrelated codeValidate()event name must matchRaiseCustomEvent()event nameRaiseCustomEvent()event name should match the containing method nameMotivation
Event sink helpers follow a pattern where
Validate("EventName")checks if the event should fire, andRaiseCustomEvent("EventName")actually fires it. A mismatch between these names is a subtle bug that can cause events to never fire or fire incorrectly. This analyzer catches these issues at compile time.Test plan
NetOffice.AnalyzersprojectValidate("Foo")is paired withRaiseCustomEvent("Bar")