diff --git a/.gitignore b/.gitignore index 1ab415f5..61213035 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ node_modules/ dist/ *.tgz +.history diff --git a/README.md b/README.md index dae8f3a8..223c8131 100644 --- a/README.md +++ b/README.md @@ -167,5 +167,6 @@ CLI option\ | [valid-describe-callback](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-describe-callback.md) | Enforce valid `describe()` callback | ✅ | | | | [valid-expect-in-promise](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect-in-promise.md) | Require promises that have expectations in their chain to be valid | ✅ | | | | [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 | ✅ | 🔧 | | +| [valid-test-annotations](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-test-annotations.md) | Enforce valid annotation format in test blocks | ✅ | | | | [valid-test-tags](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-test-tags.md) | Enforce valid tag format in test blocks | ✅ | | | +| [valid-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-title.md) | Enforce valid titles | ✅ | 🔧 | | diff --git a/docs/rules/valid-test-annotations.md b/docs/rules/valid-test-annotations.md new file mode 100644 index 00000000..ca09cf6c --- /dev/null +++ b/docs/rules/valid-test-annotations.md @@ -0,0 +1,140 @@ +# Valid Test Annotations + +This rule ensures that test annotations in Playwright test files follow the +correct format according to Playwright's annotation API. + +## Rule Details + +This rule enforces the following: + +1. Annotations must be objects with a `type` property (not strings, numbers, or + other primitives) +2. The `type` property must be a string +3. The optional `description` property must be a string +4. Only `type` and `description` properties are allowed in annotation objects +5. Tags should not be placed inside annotations (use the separate `tag` property + instead) + +### Examples + +```ts +// Valid +test('my test', { annotation: { type: 'issue' } }, async ({ page }) => {}) + +test( + 'my test', + { + annotation: { + type: 'issue', + description: 'https://github.com/microsoft/playwright/issues/23180', + }, + }, + async ({ page }) => {}, +) + +// Valid with array of annotations +test( + 'my test', + { + annotation: [ + { type: 'issue', description: 'BUG-123' }, + { type: 'performance' }, + ], + }, + async ({ page }) => {}, +) + +// Valid with tag (separate properties) +test( + 'my test', + { + tag: '@e2e', + annotation: { type: 'issue', description: 'BUG-123' }, + }, + async ({ page }) => {}, +) + +// Valid in test.describe +test.describe( + 'group', + { annotation: { type: 'category', description: 'report' } }, + () => {}, +) + +// Valid in test.skip, test.fixme, test.only +test.skip( + 'my test', + { annotation: { type: 'issue', description: 'BUG-123' } }, + async ({ page }) => {}, +) + +// Invalid - string instead of object +test('my test', { annotation: 'bug' }, async ({ page }) => {}) + +// Invalid - number instead of object +test('my test', { annotation: 123 }, async ({ page }) => {}) + +// Invalid - array with string instead of object +test('my test', { annotation: ['bug'] }, async ({ page }) => {}) + +// Invalid - tag property inside annotation +test( + 'my test', + { annotation: { tag: ['@XRAY-123'] } }, + async ({ page }) => {}, +) + +// Invalid - missing type property +test( + 'my test', + { annotation: { description: 'some description' } }, + async ({ page }) => {}, +) + +// Invalid - type is not a string +test( + 'my test', + { annotation: { type: 123, description: 'desc' } }, + async ({ page }) => {}, +) + +// Invalid - description is not a string +test( + 'my test', + { annotation: { type: 'issue', description: 123 } }, + async ({ page }) => {}, +) + +// Invalid - unknown property +test( + 'my test', + { + annotation: { type: 'issue', description: 'desc', invalid: true }, + }, + async ({ page }) => {}, +) +``` + +## Rationale + +Playwright's annotation API expects annotations to be objects with a `type` +property and an optional `description` property. Using incorrect formats can +lead to: + +1. Annotations not appearing in test reports +2. Confusion between tags and annotations +3. Runtime errors or unexpected behavior + +Common mistakes this rule prevents: + +- Using strings like `annotation: "bug"` instead of + `annotation: { type: "bug" }` +- Mixing up tags and annotations by putting `tag` inside `annotation` +- Missing the required `type` property +- Using non-string values for `type` or `description` + +## Further Reading + +- [Playwright Test Annotations Documentation](https://playwright.dev/docs/test-annotations) +- [Playwright Test Tags Documentation](https://playwright.dev/docs/test-annotations#tag-tests) + diff --git a/eslint.config.js b/eslint.config.js index aa3aa1b8..9b16b059 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -3,7 +3,7 @@ import mskelton from '@mskelton/eslint-config' export default [ ...mskelton.recommended, { - ignores: ['dist', 'examples'], + ignores: ['dist', 'examples', '.history'], }, { files: ['**/*.test.ts'], diff --git a/src/index.ts b/src/index.ts index 0101521a..1fca480c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -48,6 +48,7 @@ import requireTopLevelDescribe from './rules/require-top-level-describe.js' import validDescribeCallback from './rules/valid-describe-callback.js' import validExpect from './rules/valid-expect.js' import validExpectInPromise from './rules/valid-expect-in-promise.js' +import validTestAnnotations from './rules/valid-test-annotations.js' import validTestTags from './rules/valid-test-tags.js' import validTitle from './rules/valid-title.js' @@ -103,6 +104,7 @@ const index = { 'valid-describe-callback': validDescribeCallback, 'valid-expect': validExpect, 'valid-expect-in-promise': validExpectInPromise, + 'valid-test-annotations': validTestAnnotations, 'valid-test-tags': validTestTags, 'valid-title': validTitle, }, @@ -135,6 +137,7 @@ const sharedConfig = { 'playwright/valid-describe-callback': 'error', 'playwright/valid-expect': 'error', 'playwright/valid-expect-in-promise': 'error', + 'playwright/valid-test-annotations': 'error', 'playwright/valid-test-tags': 'error', 'playwright/valid-title': 'error', }, diff --git a/src/rules/valid-test-annotations.test.ts b/src/rules/valid-test-annotations.test.ts new file mode 100644 index 00000000..cf4e275d --- /dev/null +++ b/src/rules/valid-test-annotations.test.ts @@ -0,0 +1,97 @@ +import { runRuleTester } from '../utils/rule-tester.js' +import rule from './valid-test-annotations.js' + +runRuleTester('valid-test-annotations', rule, { + invalid: [ + // String annotation instead of object + { + code: 'test("my test", { annotation: "bug" }, async ({ page }) => {})', + errors: [{ messageId: 'invalidAnnotationFormat' }], + }, + // Number annotation + { + code: 'test("my test", { annotation: 123 }, async ({ page }) => {})', + errors: [{ messageId: 'invalidAnnotationFormat' }], + }, + // Array with string instead of object + { + code: 'test("my test", { annotation: ["bug"] }, async ({ page }) => {})', + errors: [{ messageId: 'invalidAnnotationFormat' }], + }, + // Object with tag property (should use tag, not annotation) + { + code: 'test("my test", { annotation: { tag: ["@XRAY-123"] } }, async ({ page }) => {})', + errors: [{ messageId: 'tagInAnnotation' }], + }, + // Object with tag property in array + { + code: 'test("my test", { annotation: [{ tag: ["@XRAY-123"], annotation: "bug" }] }, async ({ page }) => {})', + errors: [{ messageId: 'tagInAnnotation' }], + }, + // Missing type property + { + code: 'test("my test", { annotation: { description: "some description" } }, async ({ page }) => {})', + errors: [{ messageId: 'missingType' }], + }, + // Type is not a string + { + code: 'test("my test", { annotation: { type: 123, description: "desc" } }, async ({ page }) => {})', + errors: [{ messageId: 'invalidTypeValue' }], + }, + // Description is not a string + { + code: 'test("my test", { annotation: { type: "issue", description: 123 } }, async ({ page }) => {})', + errors: [{ messageId: 'invalidDescriptionValue' }], + }, + // Invalid property in annotation object + { + code: 'test("my test", { annotation: { type: "issue", description: "desc", invalid: true } }, async ({ page }) => {})', + errors: [ + { + data: { property: 'invalid' }, + messageId: 'invalidAnnotationProperty', + }, + ], + }, + // Multiple errors in array + { + code: 'test("my test", { annotation: [{ type: "issue" }, { description: "no type" }] }, async ({ page }) => {})', + errors: [{ messageId: 'missingType' }], + }, + // test.describe with invalid annotation + { + code: 'test.describe("group", { annotation: "bug" }, () => {})', + errors: [{ messageId: 'invalidAnnotationFormat' }], + }, + // test.skip with invalid annotation + { + code: 'test.skip("my test", { annotation: { tag: "@bug" } }, async ({ page }) => {})', + errors: [{ messageId: 'tagInAnnotation' }], + }, + ], + valid: [ + // Valid annotation with type only + 'test("my test", { annotation: { type: "issue" } }, async ({ page }) => {})', + // Valid annotation with type and description + 'test("my test", { annotation: { type: "issue", description: "https://github.com/microsoft/playwright/issues/23180" } }, async ({ page }) => {})', + // Valid annotation array + 'test("my test", { annotation: [{ type: "issue", description: "BUG-123" }, { type: "performance" }] }, async ({ page }) => {})', + // Valid annotation with tag (separate properties) + 'test("my test", { tag: "@e2e", annotation: { type: "issue", description: "BUG-123" } }, async ({ page }) => {})', + // Valid annotation in test.describe + 'test.describe("group", { annotation: { type: "category", description: "report" } }, () => {})', + // Valid annotation in test.skip + 'test.skip("my test", { annotation: { type: "issue", description: "BUG-123" } }, async ({ page }) => {})', + // Valid annotation in test.only + 'test.only("my test", { annotation: { type: "issue" } }, async ({ page }) => {})', + // No annotation property + 'test("my test", async ({ page }) => {})', + // Only tag property + 'test("my test", { tag: "@e2e" }, async ({ page }) => {})', + // Empty options object + 'test("my test", {}, async ({ page }) => {})', + // test.step with annotation + 'test.step("step", { annotation: { type: "issue" } }, async () => {})', + ], +}) + diff --git a/src/rules/valid-test-annotations.ts b/src/rules/valid-test-annotations.ts new file mode 100644 index 00000000..21ed048e --- /dev/null +++ b/src/rules/valid-test-annotations.ts @@ -0,0 +1,161 @@ +import { TSESTree } from '@typescript-eslint/utils' +import ESTree from 'estree' +import { createRule } from '../utils/createRule.js' +import { parseFnCall } from '../utils/parseFnCall.js' + +export default createRule({ + create(context) { + const validateAnnotationObject = ( + annotationObj: TSESTree.ObjectExpression, + reportNode: ESTree.Node, + ) => { + let hasType = false + const validProperties = new Set(['type', 'description']) + + for (const prop of annotationObj.properties) { + if (prop.type !== 'Property' || 'argument' in prop) { + continue + } + + if (prop.key.type !== 'Identifier') { + continue + } + + const propName = prop.key.name + + // Check for 'tag' property in annotation + if (propName === 'tag') { + context.report({ + messageId: 'tagInAnnotation', + node: reportNode, + }) + return + } + + // Check for invalid properties + if (!validProperties.has(propName)) { + context.report({ + data: { property: propName }, + messageId: 'invalidAnnotationProperty', + node: reportNode, + }) + return + } + + // Validate 'type' property + if (propName === 'type') { + hasType = true + if ( + prop.value.type !== 'Literal' || + typeof prop.value.value !== 'string' + ) { + context.report({ + messageId: 'invalidTypeValue', + node: reportNode, + }) + return + } + } + + // Validate 'description' property + if (propName === 'description') { + if ( + prop.value.type !== 'Literal' || + typeof prop.value.value !== 'string' + ) { + context.report({ + messageId: 'invalidDescriptionValue', + node: reportNode, + }) + return + } + } + } + + // Check if 'type' property is present + if (!hasType) { + context.report({ + messageId: 'missingType', + node: reportNode, + }) + } + } + + return { + CallExpression(node) { + const call = parseFnCall(context, node) + if (!call) return + + const { type } = call + if (type !== 'test' && type !== 'describe' && type !== 'step') return + + // Check if there's an options object as the second argument + if (node.arguments.length < 2) return + const optionsArg = node.arguments[1] + if (!optionsArg || optionsArg.type !== 'ObjectExpression') return + + // Look for the annotation property in the options object + const annotationProperty = optionsArg.properties.find( + (prop) => + prop.type === 'Property' && + !('argument' in prop) && + prop.key.type === 'Identifier' && + prop.key.name === 'annotation', + ) as TSESTree.Property | undefined + + if (!annotationProperty) return + + const annotationValue = annotationProperty.value + + // Annotation must be an object or array of objects + if (annotationValue.type === 'ObjectExpression') { + validateAnnotationObject(annotationValue, optionsArg) + } else if (annotationValue.type === 'ArrayExpression') { + // Validate each element in the array + for (const element of annotationValue.elements) { + if (!element) continue + + if (element.type !== 'ObjectExpression') { + context.report({ + messageId: 'invalidAnnotationFormat', + node: optionsArg, + }) + return + } + + validateAnnotationObject(element, optionsArg) + } + } else { + // Invalid annotation format (string, number, etc.) + context.report({ + messageId: 'invalidAnnotationFormat', + node: optionsArg, + }) + } + }, + } + }, + meta: { + docs: { + category: 'Best Practices', + description: + 'Enforce valid annotation format in Playwright test blocks', + recommended: true, + url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-test-annotations.md', + }, + messages: { + invalidAnnotationFormat: + 'Annotation must be an object with "type" property or an array of such objects', + invalidAnnotationProperty: + 'Invalid property "{{property}}" in annotation. Only "type" and "description" are allowed', + invalidDescriptionValue: 'Annotation "description" must be a string', + invalidTypeValue: 'Annotation "type" must be a string', + missingType: 'Annotation must have a "type" property', + tagInAnnotation: + 'Use "tag" property for tags, not inside "annotation". Tags should be defined separately', + }, + schema: [], + type: 'problem', + }, +}) +