Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Commit

Permalink
Check for non-public modifiers in constructors (#4511) (#4619)
Browse files Browse the repository at this point in the history
This commit checks the modifiers of each constructor declaration. If it has any modifier that isn't `public`, then it's actually necessary and shouldn't be flagged by unnecessary-constructor rule.
  • Loading branch information
ericbf authored and Josh Goldberg committed Apr 1, 2019
1 parent e544769 commit 613c311
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 11 deletions.
19 changes: 10 additions & 9 deletions src/rules/unnecessaryConstructorRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,23 @@ export class Rule extends Lint.Rules.AbstractRule {
const isEmptyConstructor = (node: ts.ConstructorDeclaration): boolean =>
node.body !== undefined && node.body.statements.length === 0;

const containsConstructorParameter = (node: ts.ConstructorDeclaration): boolean => {
for (const parameter of node.parameters) {
if (isParameterProperty(parameter)) {
return true;
}
}
const containsConstructorParameter = (node: ts.ConstructorDeclaration): boolean =>
// If this has any parameter properties
node.parameters.some(isParameterProperty);

return false;
};
const isAccessRestrictingConstructor = (node: ts.ConstructorDeclaration): boolean =>
// No modifiers implies public
node.modifiers != undefined &&
// If this has any modifier that isn't public, it's doing something
node.modifiers.some(modifier => modifier.kind !== ts.SyntaxKind.PublicKeyword);

function walk(context: Lint.WalkContext) {
const callback = (node: ts.Node): void => {
if (
isConstructorDeclaration(node) &&
isEmptyConstructor(node) &&
!containsConstructorParameter(node)
!containsConstructorParameter(node) &&
!isAccessRestrictingConstructor(node)
) {
context.addFailureAtNode(node, Rule.FAILURE_STRING);
} else {
Expand Down
10 changes: 8 additions & 2 deletions test/rules/unnecessary-constructor/test.ts.lint
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ class PublicConstructor {

class ProtectedConstructor {
protected constructor() { }
~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
}

class PrivateConstructor {
private constructor() { }
~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
}

class SameLine { constructor() { } }
Expand Down Expand Up @@ -55,6 +53,14 @@ class ContainsParameter {
~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
}

class PrivateContainsParameter {
private constructor(x: number) { }
}

class ProtectedContainsParameter {
protected constructor(x: number) { }
}

class ContainsParameterDeclaration {
constructor(public x: number) { }
}
Expand Down

0 comments on commit 613c311

Please sign in to comment.