Skip to content

Commit

Permalink
fix(valid-title): Accept string variables as a valid title
Browse files Browse the repository at this point in the history
Fixes #295
Fixes #243
Fixes #312
Fixes #320

I give in, I've got enough reports of this from users I'm tired of
dealing with it and decided to just support basic semantic analysis. No
doubt at some point someone is going to expect it to work with full type
information cause they'll have some weird use case where they import
some title string function from a util or something dumb like that, but
at least this will quell the nonsense for most simple cases.
  • Loading branch information
mskelton committed Oct 19, 2024
1 parent 79a9681 commit ef8cfa5
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 17 deletions.
6 changes: 6 additions & 0 deletions docs/rules/valid-title.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,19 @@ test(123, () => {})
test.describe(String(/.+/), () => {})
test.describe(myFunction, () => {})
test.describe(6, function () {})

const title = 123
test(title, () => {})
```

Examples of **correct** code for this rule:

```javascript
test('is a string', () => {})
test.describe('is a string', () => {})

const title = 'is a string'
test(title, () => {})
```

Examples of **correct** code when `ignoreTypeOfDescribeName` is `true`:
Expand Down
48 changes: 33 additions & 15 deletions src/rules/valid-title.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,17 +607,6 @@ runRuleTester('title-must-be-string', rule, {
],
options: [{ ignoreTypeOfTestName: false }],
},
{
code: 'const foo = "my-title"; test(foo, () => {});',
errors: [
{
column: 30,
line: 1,
messageId: 'titleMustBeString',
},
],
options: [{ ignoreTypeOfTestName: false }],
},
{
code: 'test(123, () => {});',
errors: [
Expand Down Expand Up @@ -731,6 +720,22 @@ runRuleTester('title-must-be-string', rule, {
],
options: [{ ignoreTypeOfStepName: false }],
},
// Basic semantic analysis
{
code: javascript`
const title = 123;
test(title, () => {
expect(1).toBe(1);
});
`,
errors: [
{
column: 15,
line: 1,
messageId: 'titleMustBeString',
},
],
},
// Global aliases
{
code: 'it(String(/.+/), () => {});',
Expand Down Expand Up @@ -768,10 +773,6 @@ runRuleTester('title-must-be-string', rule, {
code: 'test(String(/.+/), () => {});',
options: [{ ignoreTypeOfTestName: true }],
},
{
code: 'const foo = "my-title"; test(foo, () => {});',
options: [{ ignoreTypeOfTestName: true }],
},
{
code: 'test.describe(myFunction, () => {});',
options: [{ ignoreTypeOfDescribeName: true }],
Expand All @@ -780,6 +781,23 @@ runRuleTester('title-must-be-string', rule, {
code: 'test.describe(skipFunction, () => {});',
options: [{ disallowedWords: [], ignoreTypeOfDescribeName: true }],
},
// Basic semantic analysis
{
code: 'const foo = "my-title"; test(foo, () => {});',
options: [{ ignoreTypeOfTestName: true }],
},
{
code: 'const foo = "my-title"; test(foo, () => {});',
options: [{ ignoreTypeOfTestName: false }],
},
{
code: javascript`
const title = "is a string";
test(title, () => {
expect(1).toBe(1);
});
`,
},
// Global aliases
{
code: 'it("is a string", () => {});',
Expand Down
12 changes: 10 additions & 2 deletions src/rules/valid-title.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import ESTree from 'estree'
import { getStringValue, isStringNode, StringNode } from '../utils/ast'
import {
dereference,
getStringValue,
isStringNode,
StringNode,
} from '../utils/ast'
import { createRule } from '../utils/createRule'
import { parseFnCall } from '../utils/parseFnCall'

Expand Down Expand Up @@ -118,9 +123,12 @@ export default createRule({
return
}

const [argument] = node.arguments
let argument: ESTree.Node = node.arguments[0]
if (!argument) return

// Attempt to dereference the argument if it's a variable
argument = dereference(context, argument) ?? argument

if (!isStringNode(argument)) {
if (
argument.type === 'BinaryExpression' &&
Expand Down

0 comments on commit ef8cfa5

Please sign in to comment.