feat: Add valid-title rule (#177)

* feat: Add `valid-title` rule

* Cleanup

README.md

@@ -173,3 +173,4 @@ command line option.\
 |     |     |     | [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md)   | Require test cases and hooks to be inside a `test.describe` block |
 |     | 🔧  |     | [require-soft-assertions](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-soft-assertions.md)         | Require assertions to use `expect.soft()`                         |
 | ✔  |     |     | [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md)                               | Enforce valid `expect()` usage                                    |
+| ✔  | 🔧  |     | [valid-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-title.md)                                 | Enforce valid titles                                              |

docs/rules/require-top-level-describe.md

@@ -62,7 +62,7 @@ using the `maxTopLevelDescribes` option:
 
 Examples of **incorrect** code with the above config:
 
-```js
+```javascript
 test.describe('test suite', () => {
   test('test', () => {});
 });

docs/rules/valid-title.md

@@ -0,0 +1,250 @@
+# Enforce valid titles (`valid-title`)
+
+Checks that the title of test blocks are valid by ensuring that titles are:
+
+- not empty,
+- is a string,
+- not prefixed with their block name,
+- have no leading or trailing spaces
+
+## Rule details
+
+**emptyTitle**
+
+An empty title is not informative, and serves little purpose.
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+test.describe('', () => {});
+test.describe('foo', () => {
+  test('', () => {});
+});
+test('', () => {});
+```
+
+Examples of **correct** code for this rule:
+
+```javascript
+test.describe('foo', () => {});
+test.describe('foo', () => {
+  test('bar', () => {});
+});
+test('foo', () => {});
+```
+
+**titleMustBeString**
+
+Titles for `describe` and `test` blocks should always be a string; you can
+disable this with the `ignoreTypeOfDescribeName` and `ignoreTypeOfTestName`
+options.
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+test(123, () => {});
+test.describe(String(/.+/), () => {});
+test.describe(myFunction, () => {});
+test.describe(6, function () {});
+```
+
+Examples of **correct** code for this rule:
+
+```javascript
+test('is a string', () => {});
+test.describe('is a string', () => {});
+```
+
+Examples of **correct** code when `ignoreTypeOfDescribeName` is `true`:
+
+```javascript
+test('is a string', () => {});
+test.describe('is a string', () => {});
+
+test.describe(String(/.+/), () => {});
+test.describe(myFunction, () => {});
+test.describe(6, function () {});
+```
+
+Examples of **correct** code when `ignoreTypeOfTestName` is `true`:
+
+```javascript
+const myTestName = 'is a string';
+
+test(String(/.+/), () => {});
+test(myFunction, () => {});
+test(myTestName, () => {});
+test(6, function () {});
+```
+
+**duplicatePrefix**
+
+A `describe` / `test` block should not start with `duplicatePrefix`
+
+Examples of **incorrect** code for this rule
+
+```javascript
+test('test foo', () => {});
+
+test.describe('foo', () => {
+  test('test bar', () => {});
+});
+
+test.describe('describe foo', () => {
+  test('bar', () => {});
+});
+```
+
+Examples of **correct** code for this rule
+
+```javascript
+test('foo', () => {});
+
+test.describe('foo', () => {
+  test('bar', () => {});
+});
+```
+
+**accidentalSpace**
+
+A `describe` / `test` block should not contain accidentalSpace, but can be
+turned off via the `ignoreSpaces` option:
+
+Examples of **incorrect** code for this rule
+
+```javascript
+test(' foo', () => {});
+
+test.describe('foo', () => {
+  test(' bar', () => {});
+});
+
+test.describe(' foo', () => {
+  test('bar', () => {});
+});
+
+test.describe('foo  ', () => {
+  test('bar', () => {});
+});
+```
+
+Examples of **correct** code for this rule
+
+```javascript
+test('foo', () => {});
+
+test.describe('foo', () => {
+  test('bar', () => {});
+});
+```
+
+## Options
+
+```ts
+interface Options {
+  ignoreSpaces?: boolean;
+  ignoreTypeOfDescribeName?: boolean;
+  disallowedWords?: string[];
+  mustNotMatch?: Partial<Record<'describe' | 'test', string>> | string;
+  mustMatch?: Partial<Record<'describe' | 'test', string>> | string;
+}
+```
+
+#### `ignoreSpaces`
+
+Default: `false`
+
+When enabled, the leading and trailing spaces won't be checked.
+
+#### `ignoreTypeOfDescribeName`
+
+Default: `false`
+
+When enabled, the type of the first argument to `describe` blocks won't be
+checked.
+
+#### `disallowedWords`
+
+Default: `[]`
+
+A string array of words that are not allowed to be used in test titles. Matching
+is not case-sensitive, and looks for complete words:
+
+Examples of **incorrect** code when using `disallowedWords`:
+
+```javascript
+// with disallowedWords: ['correct', 'all', 'every', 'properly']
+test.describe('the correct way to do things', () => {});
+test.describe('every single one of them', () => {});
+test('has ALL the things', () => {});
+test(`that the value is set properly`, () => {});
+```
+
+Examples of **correct** code when using `disallowedWords`:
+
+```javascript
+// with disallowedWords: ['correct', 'all', 'every', 'properly']
+test('correctly sets the value', () => {});
+test('that everything is as it should be', () => {});
+test.describe('the proper way to handle things', () => {});
+```
+
+#### `mustMatch` & `mustNotMatch`
+
+Defaults: `{}`
+
+Allows enforcing that titles must match or must not match a given Regular
+Expression, with an optional message. An object can be provided to apply
+different Regular Expressions (with optional messages) to specific Playwright
+test function groups (`describe`, `test`).
+
+Examples of **incorrect** code when using `mustMatch`:
+
+```javascript
+// with mustMatch: '^that'
+test.describe('the correct way to do things', () => {});
+test('this there!', () => {});
+
+// with mustMatch: { test: '^that' }
+test.describe('the tests that will be run', () => {});
+test('the stuff works', () => {});
+test('errors that are thrown have messages', () => {});
+```
+
+Examples of **correct** code when using `mustMatch`:
+
+```javascript
+// with mustMatch: '^that'
+test.describe('that thing that needs to be done', () => {});
+test('that this there!', () => {});
+
+// with mustMatch: { test: '^that' }
+test.describe('the tests will be run', () => {});
+test('that the stuff works', () => {});
+```
+
+Optionally you can provide a custom message to show for a particular matcher by
+using a tuple at any level where you can provide a matcher:
+
+```javascript
+const prefixes = ['when', 'with', 'without', 'if', 'unless', 'for'];
+const prefixesList = prefixes.join('  - \n');
+
+module.exports = {
+  rules: {
+    'playwright/valid-title': [
+      'error',
+      {
+        mustNotMatch: ['\\.$', 'Titles should not end with a full-stop'],
+        mustMatch: {
+          describe: [
+            new RegExp(`^(?:[A-Z]|\\b(${prefixes.join('|')})\\b`, 'u').source,
+            `Describe titles should either start with a capital letter or one of the following prefixes: ${prefixesList}`,
+          ],
+          test: /[^A-Z]/u.source,
+        },
+      },
+    ],
+  },
+};
+```

src/index.ts

@@ -27,6 +27,7 @@ import preferWebFirstAssertions from './rules/prefer-web-first-assertions';
 import requireSoftAssertions from './rules/require-soft-assertions';
 import requireTopLevelDescribe from './rules/require-top-level-describe';
 import validExpect from './rules/valid-expect';
+import validTitle from './rules/valid-title';
 
 const index = {
   configs: {},
@@ -59,6 +60,7 @@ const index = {
     'require-soft-assertions': requireSoftAssertions,
     'require-top-level-describe': requireTopLevelDescribe,
     'valid-expect': validExpect,
+    'valid-title': validTitle,
   },
 };
 
@@ -82,6 +84,7 @@ const sharedConfig = {
     'playwright/no-wait-for-timeout': 'warn',
     'playwright/prefer-web-first-assertions': 'error',
     'playwright/valid-expect': 'error',
+    'playwright/valid-title': 'error',
   },
 };
 

src/rules/expect-expect.ts

@@ -1,6 +1,6 @@
 import { Rule } from 'eslint';
 import * as ESTree from 'estree';
-import { dig, isExpectCall, isTest } from '../utils/ast';
+import { dig, isExpectCall, isTestCall } from '../utils/ast';
 import { getAdditionalAssertFunctionNames } from '../utils/misc';
 
 function isAssertionCall(
@@ -33,7 +33,7 @@ export default {
 
     return {
       CallExpression(node) {
-        if (isTest(node, ['fixme', 'only', 'skip'])) {
+        if (isTestCall(node, ['fixme', 'only', 'skip'])) {
           unchecked.push(node);
         } else if (isAssertionCall(node, additionalAssertFunctionNames)) {
           checkExpressions(context.sourceCode.getAncestors(node));

src/rules/no-conditional-in-test.ts

@@ -1,12 +1,12 @@
 import { Rule } from 'eslint';
-import { findParent, isTest } from '../utils/ast';
+import { findParent, isTestCall } from '../utils/ast';
 
 export default {
   create(context) {
     function checkConditional(node: Rule.Node & Rule.NodeParentExtension) {
       const call = findParent(node, 'CallExpression');
 
-      if (call && isTest(call)) {
+      if (call && isTestCall(call)) {
         context.report({ messageId: 'conditionalInTest', node });
       }
     }

src/rules/no-focused-test.ts

@@ -1,12 +1,12 @@
 import { Rule } from 'eslint';
-import { isDescribeCall, isPropertyAccessor, isTest } from '../utils/ast';
+import { isDescribeCall, isPropertyAccessor, isTestCall } from '../utils/ast';
 
 export default {
   create(context) {
     return {
       CallExpression(node) {
         if (
-          (isTest(node) || isDescribeCall(node)) &&
+          (isTestCall(node) || isDescribeCall(node)) &&
           node.callee.type === 'MemberExpression' &&
           isPropertyAccessor(node.callee, 'only')
         ) {

src/rules/no-skipped-test.ts

@@ -2,7 +2,7 @@ import { Rule } from 'eslint';
 import {
   isDescribeCall,
   isPropertyAccessor,
-  isTest,
+  isTestCall,
   isTestIdentifier,
 } from '../utils/ast';
 
@@ -19,7 +19,7 @@ export default {
           callee.type === 'MemberExpression' &&
           isPropertyAccessor(callee, 'skip')
         ) {
-          const isHook = isTest(node) || isDescribeCall(node);
+          const isHook = isTestCall(node) || isDescribeCall(node);
 
           // If allowConditional is enabled and it's not a test/describe hook,
           // we ignore any `test.skip` calls that have no arguments.

src/rules/prefer-lowercase-title.ts

@@ -3,18 +3,12 @@ import * as ESTree from 'estree';
 import {
   getStringValue,
   isDescribeCall,
-  isStringLiteral,
-  isTest,
+  isStringNode,
+  isTestCall,
 } from '../utils/ast';
 
 type Method = 'test' | 'test.describe';
 
-function isString(
-  node: ESTree.Expression | ESTree.SpreadElement,
-): node is ESTree.Literal | ESTree.TemplateLiteral {
-  return node && (isStringLiteral(node) || node.type === 'TemplateLiteral');
-}
-
 export default {
   create(context) {
     const { allowedPrefixes, ignore, ignoreTopLevelDescribe } = {
@@ -30,7 +24,7 @@ export default {
       CallExpression(node) {
         const method = isDescribeCall(node)
           ? 'test.describe'
-          : isTest(node)
+          : isTestCall(node)
           ? 'test'
           : null;
 
@@ -45,7 +39,7 @@ export default {
         }
 
         const [title] = node.arguments;
-        if (!isString(title)) {
+        if (!isStringNode(title)) {
           return;
         }