feat: add require-visibility rule

.changeset/two-adults-sleep.md

@@ -0,0 +1,5 @@
+---
+'eslint-plugin-php': minor
+---
+
+Add `require-visibility` rule

@types/php-parser.d.ts

@@ -0,0 +1,16 @@
+declare module 'php-parser' {
+	// There is a typing issue in `php-parser` that `name` is typed as `string`.
+	class Constant extends Node {
+		name: string | Identifier;
+		value: Node | string | number | boolean | null;
+	}
+
+	class Property extends Statement {
+		name: string | Identifier;
+		value: Node | null;
+		readonly: boolean;
+		nullable: boolean;
+		type: Identifier | Identifier[] | null;
+		attrGroups: AttrGroup[];
+	}
+}

README.md

@@ -40,7 +40,12 @@ export default defineConfig([
 
 ## Available Rules
 
-| Rule ID                | Description                           | Fixable? |
-| ---------------------- | ------------------------------------- | -------- |
-| `php/eqeqeq`           | Require the use of `===` and `!==`    | ❌       |
-| `php/no-array-keyword` | Disallow the use of the array keyword | ✅       |
+🔧 - Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/use/command-line-interface#--fix).
+
+💡 - Manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions).
+
+| Rule ID                  | Description                                         | 🔧  | 💡  |
+| ------------------------ | --------------------------------------------------- | --- | --- |
+| `php/eqeqeq`             | Require the use of `===` and `!==`                  |     |     |
+| `php/no-array-keyword`   | Disallow the use of the array keyword               | 🔧  |     |
+| `php/require-visibility` | Require visibility for class methods and properties |     | 💡  |

src/index.ts

@@ -3,6 +3,7 @@ import { PHPLanguage } from './language/php-language';
 
 import { eqeqeq } from './rules/eqeqeq';
 import { noArrayKeyword } from './rules/no-array-keyword';
+import { requireVisibility } from './rules/require-visibility';
 
 const plugin = {
 	meta: {
@@ -15,6 +16,7 @@ const plugin = {
 	rules: {
 		eqeqeq,
 		'no-array-keyword': noArrayKeyword,
+		'require-visibility': requireVisibility,
 	},
 } satisfies ESLint.Plugin;
 

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

@@ -0,0 +1,154 @@
+import { RuleTester, type Rule } from 'eslint';
+import php from '../../index';
+import { requireVisibility, visibilityOptions } from '../require-visibility';
+
+const ruleTester = new RuleTester({
+	plugins: {
+		php,
+	},
+	language: 'php/php',
+});
+
+// TODO: Fix the types.
+ruleTester.run(
+	'require-visibility',
+	requireVisibility as unknown as Rule.RuleModule,
+	{
+		valid: [
+			'<?php class Foo { public function bar() {} }',
+			'<?php class Foo { public static function bar() {} }',
+			'<?php class Foo { abstract public static function bar(); }',
+			'<?php class Foo { public $a = 1; }',
+			'<?php class Foo { public static $a = 1; }',
+			'<?php class Foo { private const FOO = 1; }',
+			'<?php function foo() {}',
+		],
+		invalid: [
+			{
+				name: 'method',
+				code: '<?php class Foo { function bar() {} }',
+				errors: [
+					{
+						messageId: 'requireVisibilityForMethod',
+						data: {
+							name: 'bar',
+						},
+						line: 1,
+						column: 19,
+						endLine: 1,
+						endColumn: 36,
+						suggestions: [
+							{
+								messageId: 'addVisibility',
+								data: { visibility: 'private' },
+								output: '<?php class Foo { private function bar() {} }',
+							},
+							{
+								messageId: 'addVisibility',
+								data: { visibility: 'protected' },
+								output: '<?php class Foo { protected function bar() {} }',
+							},
+							{
+								messageId: 'addVisibility',
+								data: { visibility: 'public' },
+								output: '<?php class Foo { public function bar() {} }',
+							},
+						],
+					},
+				],
+			},
+			{
+				name: 'single class constant',
+				code: '<?php class Foo { const BAR = 1; }',
+				errors: [
+					{
+						messageId: 'requireVisibilityForClassConstant',
+						line: 1,
+						column: 19,
+						endLine: 1,
+						endColumn: 32,
+						suggestions: visibilityOptions.map((visibility) => ({
+							messageId: 'addVisibility',
+							data: { visibility },
+							output: `<?php class Foo { ${visibility} const BAR = 1; }`,
+						})),
+					},
+				],
+			},
+			{
+				name: 'multiple class constants',
+				code: `<?php class Foo {
+							const
+								BAR = 1,
+								BAZ = 2;
+						}`,
+				errors: [
+					{
+						messageId: 'requireVisibilityForClassConstants',
+						line: 2,
+						column: 8,
+						endLine: 4,
+						endColumn: 16,
+						suggestions: visibilityOptions.map((visibility) => ({
+							messageId: 'addVisibility',
+							data: { visibility },
+							output: `<?php class Foo {
+							${visibility} const
+								BAR = 1,
+								BAZ = 2;
+						}`,
+						})),
+					},
+				],
+			},
+			{
+				name: 'single class property',
+				code: '<?php class Foo { $bar = 1; }',
+				errors: [
+					{
+						messageId: 'requireVisibilityForProperty',
+						data: {
+							name: 'bar',
+						},
+						line: 1,
+						column: 19,
+						endLine: 1,
+						endColumn: 27,
+						suggestions: visibilityOptions.map((visibility) => ({
+							messageId: 'addVisibility',
+							data: { visibility },
+							output: `<?php class Foo { ${visibility} $bar = 1; }`,
+						})),
+					},
+				],
+			},
+			{
+				name: 'multiple class properties',
+				code: `<?php class Foo { 
+							$bar = 1,
+							$baz = 2;
+						}`,
+				errors: [
+					{
+						messageId: 'requireVisibilityForProperties',
+						data: {
+							name: 'bar, baz',
+						},
+						line: 2,
+						column: 8,
+						endLine: 3,
+						endColumn: 16,
+						suggestions: visibilityOptions.map((visibility) => ({
+							messageId: 'addVisibility',
+							data: { visibility },
+							output: `<?php class Foo { 
+							${visibility} $bar = 1,
+							$baz = 2;
+						}`,
+						})),
+					},
+				],
+			},
+		],
+	},
+);

src/rules/require-visibility.ts

@@ -0,0 +1,148 @@
+import { createRule } from '../utils/create-rule';
+import { extractNames } from '../utils/extract-names';
+import type { ClassConstant, Method, PropertyStatement } from 'php-parser';
+
+type MessageIds =
+	| 'requireVisibilityForMethod'
+	| 'requireVisibilityForClassConstant'
+	| 'requireVisibilityForClassConstants'
+	| 'requireVisibilityForProperty'
+	| 'requireVisibilityForProperties'
+	| 'addVisibility';
+
+type Options = [];
+
+export const visibilityOptions = ['private', 'protected', 'public'];
+
+export const requireVisibility = createRule<MessageIds, Options>({
+	meta: {
+		type: 'layout',
+		fixable: 'code',
+		hasSuggestions: true,
+		docs: {
+			description: 'Require visibility for class methods and properties',
+		},
+		messages: {
+			requireVisibilityForMethod:
+				"Visibility must be declared on method '{{name}}'.",
+
+			requireVisibilityForClassConstant:
+				"Visibility must be declared on class constant '{{name}}'.",
+
+			requireVisibilityForClassConstants:
+				"Visibility must be declared on class constants '{{name}}'.",
+
+			requireVisibilityForProperty:
+				"Visibility must be declared on property '{{name}}'.",
+
+			requireVisibilityForProperties:
+				"Visibility must be declared on properties '{{name}}'.",
+
+			addVisibility: "Add '{{visibility}}' visibility.",
+		},
+		schema: [],
+	},
+
+	create(context) {
+		return {
+			'method[visibility=""]'(_node) {
+				const node = _node as Method;
+
+				context.report({
+					node,
+					messageId: 'requireVisibilityForMethod',
+					data: {
+						name:
+							typeof node.name === 'string'
+								? node.name
+								: node.name.name,
+					},
+					suggest: visibilityOptions.map((visibility) => ({
+						messageId: 'addVisibility',
+						data: { visibility },
+						fix(fixer) {
+							const nodeText = context.sourceCode.getText(node);
+
+							return fixer.replaceText(
+								node,
+								`${visibility} ${nodeText}`,
+							);
+						},
+					})),
+				});
+			},
+
+			'classconstant[visibility=""]'(_node) {
+				const node = _node as ClassConstant;
+
+				const constKeywordLoc = context.sourceCode.findClosestKeyword(
+					node,
+					'const',
+				);
+
+				if (!constKeywordLoc) {
+					return;
+				}
+
+				const constantsNames = extractNames(node.constants);
+
+				context.report({
+					node,
+					loc: {
+						start: constKeywordLoc.start,
+						end: context.sourceCode.getLoc(node).end,
+					},
+					messageId:
+						constantsNames.length === 1
+							? 'requireVisibilityForClassConstant'
+							: 'requireVisibilityForClassConstants',
+					data: {
+						name: constantsNames.join(', '),
+					},
+					suggest: visibilityOptions.map((visibility) => ({
+						messageId: 'addVisibility',
+						data: { visibility },
+						fix(fixer) {
+							return fixer.insertTextBeforeRange(
+								[
+									constKeywordLoc.start.offset,
+									constKeywordLoc.end.offset,
+								],
+								`${visibility} `,
+							);
+						},
+					})),
+				});
+			},
+
+			'propertystatement[visibility=""]'(_node) {
+				const node = _node as PropertyStatement;
+
+				const propertiesNames = extractNames(node.properties);
+
+				context.report({
+					node,
+					messageId:
+						propertiesNames.length === 1
+							? 'requireVisibilityForProperty'
+							: 'requireVisibilityForProperties',
+					data: {
+						name: propertiesNames.join(', '),
+					},
+					suggest: visibilityOptions.map((visibility) => ({
+						messageId: 'addVisibility',
+						data: { visibility },
+						fix: (fixer) => {
+							const nodeText = context.sourceCode.getText(node);
+
+							return fixer.replaceText(
+								node,
+								`${visibility} ${nodeText}`,
+							);
+						},
+					})),
+				});
+			},
+		};
+	},
+});

src/utils/extract-names.ts

@@ -0,0 +1,11 @@
+import { Identifier } from 'php-parser';
+
+type Nameable = {
+	name: string | Identifier;
+};
+
+export function extractNames(names: Nameable[]) {
+	return names.map(({ name }) =>
+		typeof name === 'string' ? name : name.name,
+	);
+}

tsconfig.json

@@ -14,6 +14,7 @@
 
 		"module": "preserve",
 		"noEmit": true,
-		"lib": ["ESNext", "dom", "dom.iterable"]
+		"lib": ["ESNext", "dom", "dom.iterable"],
+		"typeRoots": ["./node_modules/@types", "./@types"]
 	}
 }