fix(lint/noUnusedPrivateClassMembers): handle class members declared in constructor parameters correctly#7102
Conversation
…in constructor parameters correctly
🦋 Changeset detectedLatest commit: 74d4c62 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
CodSpeed Performance ReportMerging #7102 will not alter performanceComparing Summary
|
arendjr
left a comment
There was a problem hiding this comment.
Nice work!
I think one of the diagnostics should be tweaked, but looks good otherwise! Do you want to add a changeset too?
|
|
||
| #[derive(Debug, Clone)] | ||
| pub enum UnusedMemberAction { | ||
| Remove(AnyMember), |
There was a problem hiding this comment.
nit: Maybe RemoveMember(AnyMember)? Calling it only Remove doesn't clarify how it differs from RemovePrivateModifier.
| if let Ok(AnyJsFormalParameter::JsFormalParameter(param)) = ts_property_param.formal_parameter() | ||
| { | ||
| if let Ok(binding) = param.binding() { | ||
| if let Some(identifier_binding) = binding | ||
| .as_any_js_binding() | ||
| .and_then(|b| b.as_js_identifier_binding()) | ||
| { |
There was a problem hiding this comment.
This would benefit from let chaining:
| if let Ok(AnyJsFormalParameter::JsFormalParameter(param)) = ts_property_param.formal_parameter() | |
| { | |
| if let Ok(binding) = param.binding() { | |
| if let Some(identifier_binding) = binding | |
| .as_any_js_binding() | |
| .and_then(|b| b.as_js_identifier_binding()) | |
| { | |
| if let Ok(AnyJsFormalParameter::JsFormalParameter(param)) = ts_property_param.formal_parameter() | |
| && let Ok(binding) = param.binding() | |
| && let Some(identifier_binding) = binding | |
| .as_any_js_binding() | |
| .and_then(|b| b.as_js_identifier_binding()) | |
| { |
| ``` | ||
| invalid_issue_7101.ts:9:23 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━ | ||
|
|
||
| ! This private class member is defined but never used. |
There was a problem hiding this comment.
I think the diagnostic is not correct here. It should be something such as "This argument is never used outside of the constructor."
|
This PR changed the order of diagnostics output, probably because the rule now requires |
Summary
This PR fixes #7101
privatemodifier and make it a plain method argument.no_unused_function_parameter.Note:
Previously, unused class members defined as constructor arguments were completely removed from the constructor arguments. However, this changed the constructor signature and was inconsistent with the behavior of
no_unused_function_parameter.As a side effect of this bug fix, the constructor signature is no longer changed, which has caused some existing tests to produce different results. (check changed files)
Test Plan
New test file has been added.