Skip to content

Commit c05e4cd

Browse files
authored
JS-1993 S5906: clarify that length matchers apply beyond collections (#7434)
1 parent 34ca164 commit c05e4cd

6 files changed

Lines changed: 47 additions & 12 deletions

File tree

packages/analysis/src/jsts/rules/S5906/assertion-suggestions.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,22 +229,36 @@ function buildLengthEqualitySuggestion(
229229
const actual = sourceCode.getText(actualNode);
230230
if (family === 'jest' || family === 'jasmine') {
231231
const matcher = family === 'jasmine' ? 'toHaveSize' : 'toHaveLength';
232-
return replacement(`expect(${actual})${negation(same)}.${matcher}(${expected})`, node);
232+
return replacement(
233+
`expect(${actual})${negation(same)}.${matcher}(${expected})`,
234+
node,
235+
undefined,
236+
'preferSpecificLengthAssertion',
237+
);
233238
}
234239
if (family === 'assert') {
235240
return same
236-
? replacement(`${assertObject}.lengthOf(${actual}, ${expected}${extraArguments})`, node)
241+
? replacement(
242+
`${assertObject}.lengthOf(${actual}, ${expected}${extraArguments})`,
243+
node,
244+
undefined,
245+
'preferSpecificLengthAssertion',
246+
)
237247
: null;
238248
}
239249
if (family === 'chai-should') {
240250
return replacement(
241251
`${chaiShouldReceiver(actual, actualNode)}.should${negation(same)}.have.lengthOf(${expected})`,
242252
node,
253+
undefined,
254+
'preferSpecificLengthAssertion',
243255
);
244256
}
245257
return replacement(
246258
`expect(${actual}${extraArguments}).to${negation(same)}.have.lengthOf(${expected})`,
247259
node,
260+
undefined,
261+
'preferSpecificLengthAssertion',
248262
);
249263
}
250264

packages/analysis/src/jsts/rules/S5906/assertion-utils.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { getVariableFromName, isIdentifier, isMethodCall } from '../helpers/ast.
2222
export type Suggestion = {
2323
assertion: string;
2424
replacement?: string;
25+
messageId?: 'preferSpecificAssertion' | 'preferSpecificLengthAssertion';
2526
};
2627

2728
const PLAYWRIGHT_LOCATOR_FACTORIES = new Set([
@@ -124,9 +125,11 @@ export function replacement(
124125
replacementText: string,
125126
_node: estree.CallExpression,
126127
_sourceCode?: SourceCode,
128+
messageId?: Suggestion['messageId'],
127129
): Suggestion {
128130
return {
129131
assertion: replacementText,
130132
replacement: replacementText,
133+
...(messageId ? { messageId } : {}),
131134
};
132135
}

packages/analysis/src/jsts/rules/S5906/chai-bdd.unit.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ describe('S5906', () => {
2525
messageId: 'preferSpecificAssertion',
2626
suggestions: [{ messageId: 'quickfix', output }],
2727
});
28+
const expectedLengthError = (output: string) => ({
29+
messageId: 'preferSpecificLengthAssertion',
30+
suggestions: [{ messageId: 'quickfix', output }],
31+
});
2832

2933
ruleTester.run('prefer-specific-assertions', rule, {
3034
valid: [],
@@ -106,7 +110,7 @@ describe('S5906', () => {
106110
expect(items.length).to.equal(2);
107111
`,
108112
errors: [
109-
expectedError(`
113+
expectedLengthError(`
110114
import { expect } from 'chai';
111115
112116
expect(items).to.have.lengthOf(2);

packages/analysis/src/jsts/rules/S5906/integrations.unit.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ describe('S5906', () => {
2525
messageId: 'preferSpecificAssertion',
2626
suggestions: [{ messageId: 'quickfix', output }],
2727
});
28+
const expectedLengthError = (output: string) => ({
29+
messageId: 'preferSpecificLengthAssertion',
30+
suggestions: [{ messageId: 'quickfix', output }],
31+
});
2832
const expectedErrorWithoutSuggestion = {
2933
messageId: 'preferSpecificAssertion',
3034
suggestions: [],
@@ -191,7 +195,7 @@ describe('S5906', () => {
191195
assert.strictEqual(items.length, 2, 'item count');
192196
`,
193197
errors: [
194-
expectedError(`
198+
expectedLengthError(`
195199
import { assert } from 'chai';
196200
197201
assert.lengthOf(items, 2, 'item count');

packages/analysis/src/jsts/rules/S5906/rule.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ import { getPlaywrightLocatorSuggestion } from './playwright-suggestions.js';
4646

4747
const messages = {
4848
preferSpecificAssertion:
49-
'Prefer a more specific assertion instead of this generic one, e.g. "{{assertion}}".',
49+
'Prefer "{{assertion}}" over this generic assertion; dedicated matchers read better and report clearer failures.',
50+
preferSpecificLengthAssertion:
51+
'Prefer "{{assertion}}" over this generic assertion for better reporting; it works on any object with a numeric length property.',
5052
quickfix: 'Replace with {{assertion}}.',
5153
};
5254

@@ -83,7 +85,7 @@ export const rule: Rule.RuleModule = {
8385

8486
context.report({
8587
node: node.callee,
86-
messageId: 'preferSpecificAssertion',
88+
messageId: suggestion.messageId ?? 'preferSpecificAssertion',
8789
data: { assertion: suggestion.assertion },
8890
...(suggest ? { suggest } : {}),
8991
});
@@ -210,6 +212,7 @@ function getExpectLikeSuggestion(
210212
`expect(${sourceCode.getText(actual.object)}).${negated ? 'not.' : ''}${lengthMatcher}(${expectedText})`,
211213
node,
212214
sourceCode,
215+
'preferSpecificLengthAssertion',
213216
);
214217
}
215218
const booleanExpected = getBooleanValue(expected);
@@ -370,6 +373,7 @@ function getChaiAssertSuggestion(
370373
`${assertText}.lengthOf(${sourceCode.getText(actual.object)}, ${sourceCode.getText(expected)}${trailingArgs})`,
371374
node,
372375
sourceCode,
376+
'preferSpecificLengthAssertion',
373377
);
374378
}
375379
const booleanExpected = getBooleanValue(expected);
@@ -415,6 +419,7 @@ function getChaiValueSuggestion(
415419
`expect(${sourceCode.getText(actual.object)}${messageArguments}).to${negated ? '.not' : ''}.have.lengthOf(${sourceCode.getText(expected)})`,
416420
node,
417421
sourceCode,
422+
'preferSpecificLengthAssertion',
418423
);
419424
}
420425
const booleanExpected = getBooleanValue(expected);
@@ -460,6 +465,7 @@ function getChaiShouldValueSuggestion(
460465
`${receiver}.should${negated ? '.not' : ''}.have.lengthOf(${sourceCode.getText(expected)})`,
461466
node,
462467
sourceCode,
468+
'preferSpecificLengthAssertion',
463469
);
464470
}
465471
const booleanExpected = getBooleanValue(expected);

packages/analysis/src/jsts/rules/S5906/unit.test.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ describe('S5906', () => {
4444
messageId: 'preferSpecificAssertion',
4545
suggestions: [{ messageId: 'quickfix', output }],
4646
});
47+
const expectedLengthError = (output: string) => ({
48+
messageId: 'preferSpecificLengthAssertion',
49+
suggestions: [{ messageId: 'quickfix', output }],
50+
});
4751

4852
ruleTester.run('prefer-specific-assertions', rule, {
4953
valid: [
@@ -184,7 +188,7 @@ describe('S5906', () => {
184188
`,
185189
filename: modernJasmineFixture,
186190
errors: [
187-
expectedError(`
191+
expectedLengthError(`
188192
import { expect } from 'jasmine';
189193
190194
expect(items).toHaveSize(3);
@@ -199,7 +203,7 @@ describe('S5906', () => {
199203
`,
200204
filename: mixedJasmineJestFixture,
201205
errors: [
202-
expectedError(`
206+
expectedLengthError(`
203207
import { expect } from 'jasmine';
204208
205209
expect(items).toHaveSize(3);
@@ -237,7 +241,7 @@ describe('S5906', () => {
237241
});
238242
`,
239243
errors: [
240-
expectedError(`
244+
expectedLengthError(`
241245
import { expect, test } from 'vitest';
242246
243247
test('uses generic length assertion', () => {
@@ -254,7 +258,7 @@ describe('S5906', () => {
254258
`,
255259
filename: mixedJasmineJestFixture,
256260
errors: [
257-
expectedError(`
261+
expectedLengthError(`
258262
import { expect } from 'vitest';
259263
260264
expect(items).toHaveLength(3);
@@ -338,7 +342,7 @@ describe('S5906', () => {
338342
expect(items.length === 2).toBe(true);
339343
`,
340344
errors: [
341-
expectedError(`
345+
expectedLengthError(`
342346
import { expect } from 'vitest';
343347
344348
expect(items).toHaveLength(2);
@@ -352,7 +356,7 @@ describe('S5906', () => {
352356
expect(items.length === 2).toBe(true);
353357
`,
354358
errors: [
355-
expectedError(`
359+
expectedLengthError(`
356360
import { expect } from 'jasmine';
357361
358362
expect(items).toHaveSize(2);

0 commit comments

Comments
 (0)