Skip to content
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

feat(eslint-plugin): [explicit-member-accessibility] autofix no-public #1548

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
Expand Up @@ -102,7 +102,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | |
| [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | |
| [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | :heavy_check_mark: | | |
| [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | | |
| [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | :wrench: | |
| [`@typescript-eslint/explicit-module-boundary-types`](./docs/rules/explicit-module-boundary-types.md) | Require explicit return and argument types on exported functions' and classes' public class methods | | | |
| [`@typescript-eslint/member-delimiter-style`](./docs/rules/member-delimiter-style.md) | Require a specific member delimiter style for interfaces and type literals | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | |
Expand Down
49 changes: 49 additions & 0 deletions packages/eslint-plugin/src/rules/explicit-member-accessibility.ts
@@ -1,6 +1,8 @@
import {
AST_NODE_TYPES,
TSESTree,
AST_TOKEN_TYPES,
TSESLint,
} from '@typescript-eslint/experimental-utils';
import * as util from '../util';

Expand Down Expand Up @@ -38,6 +40,7 @@ export default util.createRule<Options, MessageIds>({
// too opinionated to be recommended
recommended: false,
},
fixable: 'code',
messages: {
missingAccessibility:
'Missing accessibility modifier on {{type}} {{name}}.',
Expand Down Expand Up @@ -91,6 +94,7 @@ export default util.createRule<Options, MessageIds>({
nodeType: string,
node: TSESTree.Node,
nodeName: string,
fix: TSESLint.ReportFixFunction | null = null,
): void {
context.report({
node: node,
Expand All @@ -99,6 +103,7 @@ export default util.createRule<Options, MessageIds>({
type: nodeType,
name: nodeName,
},
fix: fix,
});
}

Expand Down Expand Up @@ -140,6 +145,7 @@ export default util.createRule<Options, MessageIds>({
nodeType,
methodDefinition,
methodName,
getUnwantedPublicAccessibilityFixer(methodDefinition),
);
} else if (check === 'explicit' && !methodDefinition.accessibility) {
reportIssue(
Expand All @@ -151,6 +157,47 @@ export default util.createRule<Options, MessageIds>({
}
}

/**
* Creates a fixer that removes a "public" keyword with following spaces
*/
function getUnwantedPublicAccessibilityFixer(
node:
| TSESTree.MethodDefinition
| TSESTree.ClassProperty
| TSESTree.TSParameterProperty,
): TSESLint.ReportFixFunction {
return function(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
const tokens = sourceCode.getTokens(node);
let rangeToRemove: TSESLint.AST.Range;
for (let i = 0; i < tokens.length; i++) {
const token = tokens[i];
if (
token.type === AST_TOKEN_TYPES.Keyword &&
token.value === 'public'
) {
const commensAfterPublicKeyword = sourceCode.getCommentsAfter(
token,
);
if (commensAfterPublicKeyword.length) {
// public /* Hi there! */ static foo()
// ^^^^^^^
rangeToRemove = [
token.range[0],
commensAfterPublicKeyword[0].range[0],
];
break;
} else {
// public static foo()
// ^^^^^^^
rangeToRemove = [token.range[0], tokens[i + 1].range[0]];
break;
}
}
}
return fixer.removeRange(rangeToRemove!);
};
}

/**
* Checks if property has an accessibility modifier.
* @param classProperty The node representing a ClassProperty.
Expand All @@ -170,6 +217,7 @@ export default util.createRule<Options, MessageIds>({
nodeType,
classProperty,
propertyName,
getUnwantedPublicAccessibilityFixer(classProperty),
);
} else if (propCheck === 'explicit' && !classProperty.accessibility) {
reportIssue(
Expand Down Expand Up @@ -217,6 +265,7 @@ export default util.createRule<Options, MessageIds>({
nodeType,
node,
nodeName,
getUnwantedPublicAccessibilityFixer(node),
);
}
break;
Expand Down
Expand Up @@ -706,5 +706,164 @@ class Test {
},
],
},
{
filename: 'test.ts',
code: `
class Test {
@public
public /*public*/constructor(private foo: string) {}
}
`,
options: [
{
accessibility: 'no-public',
},
],
errors: [
{
messageId: 'unwantedPublicAccessibility',
line: 3,
column: 3,
},
],
output: `
class Test {
@public
/*public*/constructor(private foo: string) {}
}
`,
},
{
filename: 'test.ts',
code: `
class Test {
@public
public foo() {}
}
`,
options: [
{
accessibility: 'no-public',
},
],
errors: [
{
messageId: 'unwantedPublicAccessibility',
line: 3,
column: 3,
},
],
output: `
class Test {
@public
foo() {}
}
`,
},

{
filename: 'test.ts',
code: `
class Test {
@public
public foo;
}
`,
options: [
{
accessibility: 'no-public',
},
],
errors: [
{
messageId: 'unwantedPublicAccessibility',
line: 3,
column: 3,
},
],
output: `
class Test {
@public
foo;
}
`,
},
{
filename: 'test.ts',
code: `
class Test {
public foo = "";
}
`,
options: [
{
accessibility: 'no-public',
},
],
errors: [
{
messageId: 'unwantedPublicAccessibility',
line: 3,
column: 3,
},
],
output: `
class Test {
foo = "";
}
`,
},

{
filename: 'test.ts',
code: `
class Test {
contructor(public/* Hi there */ readonly foo)
}
`,
options: [
{
accessibility: 'no-public',
overrides: { parameterProperties: 'no-public' },
},
],
errors: [
{
messageId: 'unwantedPublicAccessibility',
line: 3,
column: 14,
},
],
output: `
class Test {
contructor(/* Hi there */ readonly foo)
}
`,
},
{
filename: 'test.ts',
code: `
class Test {
contructor(public readonly foo: string)
}
`,
options: [
{
accessibility: 'no-public',
},
],
errors: [
{
messageId: 'unwantedPublicAccessibility',
line: 3,
column: 14,
},
],
output: `
class Test {
contructor(readonly foo: string)
}
`,
},
],
});