Skip to content

Commit b233df9

Browse files
NevossStyleShit
Nevoss
and
StyleShit
authored
feat: add require-visibility rule (#5)
Co-authored-by: StyleShit <32631382+StyleShit@users.noreply.github.com>
1 parent acd8e81 commit b233df9

9 files changed

Lines changed: 349 additions & 7 deletions

File tree

‎.changeset/two-adults-sleep.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'eslint-plugin-php': minor
3+
---
4+
5+
Add `require-visibility` rule

‎@types/php-parser.d.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
declare module 'php-parser' {
2+
// There is a typing issue in `php-parser` that `name` is typed as `string`.
3+
class Constant extends Node {
4+
name: string | Identifier;
5+
value: Node | string | number | boolean | null;
6+
}
7+
8+
class Property extends Statement {
9+
name: string | Identifier;
10+
value: Node | null;
11+
readonly: boolean;
12+
nullable: boolean;
13+
type: Identifier | Identifier[] | null;
14+
attrGroups: AttrGroup[];
15+
}
16+
}

‎README.md‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,12 @@ export default defineConfig([
4040

4141
## Available Rules
4242

43-
| Rule ID | Description | Fixable? |
44-
| ---------------------- | ------------------------------------- | -------- |
45-
| `php/eqeqeq` | Require the use of `===` and `!==` | ❌ |
46-
| `php/no-array-keyword` | Disallow the use of the array keyword | ✅ |
43+
🔧 - Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/use/command-line-interface#--fix).
44+
45+
💡 - Manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions).
46+
47+
| Rule ID | Description | 🔧 | 💡 |
48+
| ------------------------ | --------------------------------------------------- | --- | --- |
49+
| `php/eqeqeq` | Require the use of `===` and `!==` | | |
50+
| `php/no-array-keyword` | Disallow the use of the array keyword | 🔧 | |
51+
| `php/require-visibility` | Require visibility for class methods and properties | | 💡 |

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { PHPLanguage } from './language/php-language';
33

44
import { eqeqeq } from './rules/eqeqeq';
55
import { noArrayKeyword } from './rules/no-array-keyword';
6+
import { requireVisibility } from './rules/require-visibility';
67

78
const plugin = {
89
meta: {
@@ -15,6 +16,7 @@ const plugin = {
1516
rules: {
1617
eqeqeq,
1718
'no-array-keyword': noArrayKeyword,
19+
'require-visibility': requireVisibility,
1820
},
1921
} satisfies ESLint.Plugin;
2022

‎src/rules/__tests__/require-visibility.test.ts‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import { RuleTester, type Rule } from 'eslint';
2+
import php from '../../index';
3+
import { requireVisibility, visibilityOptions } from '../require-visibility';
4+
5+
const ruleTester = new RuleTester({
6+
plugins: {
7+
php,
8+
},
9+
language: 'php/php',
10+
});
11+
12+
// TODO: Fix the types.
13+
ruleTester.run(
14+
'require-visibility',
15+
requireVisibility as unknown as Rule.RuleModule,
16+
{
17+
valid: [
18+
'<?php class Foo { public function bar() {} }',
19+
'<?php class Foo { public static function bar() {} }',
20+
'<?php class Foo { abstract public static function bar(); }',
21+
'<?php class Foo { public $a = 1; }',
22+
'<?php class Foo { public static $a = 1; }',
23+
'<?php class Foo { private const FOO = 1; }',
24+
'<?php function foo() {}',
25+
],
26+
invalid: [
27+
{
28+
name: 'method',
29+
code: '<?php class Foo { function bar() {} }',
30+
errors: [
31+
{
32+
messageId: 'requireVisibilityForMethod',
33+
data: {
34+
name: 'bar',
35+
},
36+
line: 1,
37+
column: 19,
38+
endLine: 1,
39+
endColumn: 36,
40+
suggestions: [
41+
{
42+
messageId: 'addVisibility',
43+
data: { visibility: 'private' },
44+
output: '<?php class Foo { private function bar() {} }',
45+
},
46+
{
47+
messageId: 'addVisibility',
48+
data: { visibility: 'protected' },
49+
output: '<?php class Foo { protected function bar() {} }',
50+
},
51+
{
52+
messageId: 'addVisibility',
53+
data: { visibility: 'public' },
54+
output: '<?php class Foo { public function bar() {} }',
55+
},
56+
],
57+
},
58+
],
59+
},
60+
{
61+
name: 'single class constant',
62+
code: '<?php class Foo { const BAR = 1; }',
63+
errors: [
64+
{
65+
messageId: 'requireVisibilityForClassConstant',
66+
line: 1,
67+
column: 19,
68+
endLine: 1,
69+
endColumn: 32,
70+
suggestions: visibilityOptions.map((visibility) => ({
71+
messageId: 'addVisibility',
72+
data: { visibility },
73+
output: `<?php class Foo { ${visibility} const BAR = 1; }`,
74+
})),
75+
},
76+
],
77+
},
78+
{
79+
name: 'multiple class constants',
80+
code: `<?php class Foo {
81+
const
82+
BAR = 1,
83+
BAZ = 2;
84+
}`,
85+
errors: [
86+
{
87+
messageId: 'requireVisibilityForClassConstants',
88+
line: 2,
89+
column: 8,
90+
endLine: 4,
91+
endColumn: 16,
92+
suggestions: visibilityOptions.map((visibility) => ({
93+
messageId: 'addVisibility',
94+
data: { visibility },
95+
output: `<?php class Foo {
96+
${visibility} const
97+
BAR = 1,
98+
BAZ = 2;
99+
}`,
100+
})),
101+
},
102+
],
103+
},
104+
{
105+
name: 'single class property',
106+
code: '<?php class Foo { $bar = 1; }',
107+
errors: [
108+
{
109+
messageId: 'requireVisibilityForProperty',
110+
data: {
111+
name: 'bar',
112+
},
113+
line: 1,
114+
column: 19,
115+
endLine: 1,
116+
endColumn: 27,
117+
suggestions: visibilityOptions.map((visibility) => ({
118+
messageId: 'addVisibility',
119+
data: { visibility },
120+
output: `<?php class Foo { ${visibility} $bar = 1; }`,
121+
})),
122+
},
123+
],
124+
},
125+
{
126+
name: 'multiple class properties',
127+
code: `<?php class Foo {
128+
$bar = 1,
129+
$baz = 2;
130+
}`,
131+
errors: [
132+
{
133+
messageId: 'requireVisibilityForProperties',
134+
data: {
135+
name: 'bar, baz',
136+
},
137+
line: 2,
138+
column: 8,
139+
endLine: 3,
140+
endColumn: 16,
141+
suggestions: visibilityOptions.map((visibility) => ({
142+
messageId: 'addVisibility',
143+
data: { visibility },
144+
output: `<?php class Foo {
145+
${visibility} $bar = 1,
146+
$baz = 2;
147+
}`,
148+
})),
149+
},
150+
],
151+
},
152+
],
153+
},
154+
);

‎src/rules/require-visibility.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import { createRule } from '../utils/create-rule';
2+
import { extractNames } from '../utils/extract-names';
3+
import type { ClassConstant, Method, PropertyStatement } from 'php-parser';
4+
5+
type MessageIds =
6+
| 'requireVisibilityForMethod'
7+
| 'requireVisibilityForClassConstant'
8+
| 'requireVisibilityForClassConstants'
9+
| 'requireVisibilityForProperty'
10+
| 'requireVisibilityForProperties'
11+
| 'addVisibility';
12+
13+
type Options = [];
14+
15+
export const visibilityOptions = ['private', 'protected', 'public'];
16+
17+
export const requireVisibility = createRule<MessageIds, Options>({
18+
meta: {
19+
type: 'layout',
20+
fixable: 'code',
21+
hasSuggestions: true,
22+
docs: {
23+
description: 'Require visibility for class methods and properties',
24+
},
25+
messages: {
26+
requireVisibilityForMethod:
27+
"Visibility must be declared on method '{{name}}'.",
28+
29+
requireVisibilityForClassConstant:
30+
"Visibility must be declared on class constant '{{name}}'.",
31+
32+
requireVisibilityForClassConstants:
33+
"Visibility must be declared on class constants '{{name}}'.",
34+
35+
requireVisibilityForProperty:
36+
"Visibility must be declared on property '{{name}}'.",
37+
38+
requireVisibilityForProperties:
39+
"Visibility must be declared on properties '{{name}}'.",
40+
41+
addVisibility: "Add '{{visibility}}' visibility.",
42+
},
43+
schema: [],
44+
},
45+
46+
create(context) {
47+
return {
48+
'method[visibility=""]'(_node) {
49+
const node = _node as Method;
50+
51+
context.report({
52+
node,
53+
messageId: 'requireVisibilityForMethod',
54+
data: {
55+
name:
56+
typeof node.name === 'string'
57+
? node.name
58+
: node.name.name,
59+
},
60+
suggest: visibilityOptions.map((visibility) => ({
61+
messageId: 'addVisibility',
62+
data: { visibility },
63+
fix(fixer) {
64+
const nodeText = context.sourceCode.getText(node);
65+
66+
return fixer.replaceText(
67+
node,
68+
`${visibility} ${nodeText}`,
69+
);
70+
},
71+
})),
72+
});
73+
},
74+
75+
'classconstant[visibility=""]'(_node) {
76+
const node = _node as ClassConstant;
77+
78+
const constKeywordLoc = context.sourceCode.findClosestKeyword(
79+
node,
80+
'const',
81+
);
82+
83+
if (!constKeywordLoc) {
84+
return;
85+
}
86+
87+
const constantsNames = extractNames(node.constants);
88+
89+
context.report({
90+
node,
91+
loc: {
92+
start: constKeywordLoc.start,
93+
end: context.sourceCode.getLoc(node).end,
94+
},
95+
messageId:
96+
constantsNames.length === 1
97+
? 'requireVisibilityForClassConstant'
98+
: 'requireVisibilityForClassConstants',
99+
data: {
100+
name: constantsNames.join(', '),
101+
},
102+
suggest: visibilityOptions.map((visibility) => ({
103+
messageId: 'addVisibility',
104+
data: { visibility },
105+
fix(fixer) {
106+
return fixer.insertTextBeforeRange(
107+
[
108+
constKeywordLoc.start.offset,
109+
constKeywordLoc.end.offset,
110+
],
111+
`${visibility} `,
112+
);
113+
},
114+
})),
115+
});
116+
},
117+
118+
'propertystatement[visibility=""]'(_node) {
119+
const node = _node as PropertyStatement;
120+
121+
const propertiesNames = extractNames(node.properties);
122+
123+
context.report({
124+
node,
125+
messageId:
126+
propertiesNames.length === 1
127+
? 'requireVisibilityForProperty'
128+
: 'requireVisibilityForProperties',
129+
data: {
130+
name: propertiesNames.join(', '),
131+
},
132+
suggest: visibilityOptions.map((visibility) => ({
133+
messageId: 'addVisibility',
134+
data: { visibility },
135+
fix: (fixer) => {
136+
const nodeText = context.sourceCode.getText(node);
137+
138+
return fixer.replaceText(
139+
node,
140+
`${visibility} ${nodeText}`,
141+
);
142+
},
143+
})),
144+
});
145+
},
146+
};
147+
},
148+
});

‎src/utils/extract-names.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { Identifier } from 'php-parser';
2+
3+
type Nameable = {
4+
name: string | Identifier;
5+
};
6+
7+
export function extractNames(names: Nameable[]) {
8+
return names.map(({ name }) =>
9+
typeof name === 'string' ? name : name.name,
10+
);
11+
}

‎tsconfig.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
"module": "preserve",
1616
"noEmit": true,
17-
"lib": ["ESNext", "dom", "dom.iterable"]
17+
"lib": ["ESNext", "dom", "dom.iterable"],
18+
"typeRoots": ["./node_modules/@types", "./@types"]
1819
}
1920
}

0 commit comments

Comments
 (0)