Skip to content

Commit e5c49e7

Browse files
sonar-nigel[bot]Vibe Botclaudefrancois-mora-sonarsource
authored
JS-1177 Fix FP on S7739 where validation library 'then' config property was flagged incorrectly (#6361)
Co-authored-by: Vibe Bot <vibe-bot@sonarsource.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Francois Mora <francois.mora@sonarsource.com>
1 parent e6659a5 commit e5c49e7

4 files changed

Lines changed: 200 additions & 2 deletions

File tree

packages/jsts/src/rules/S7739/joi-project/unit.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,42 @@ describe('S7739', () => {
3333
`,
3434
filename: join(dirname, 'filename.ts'),
3535
},
36+
{
37+
// Chained joi calls - string().when() with {is, then} config
38+
code: `
39+
import Joi from 'joi';
40+
const schema = Joi.string().when('type', {
41+
is: 'email',
42+
then: Joi.string().email().required(),
43+
});
44+
`,
45+
filename: join(dirname, 'filename.ts'),
46+
},
47+
{
48+
// Joi alternatives().conditional() with switch cases containing {is, then}
49+
code: `
50+
import Joi from 'joi';
51+
const schema = Joi.alternatives().conditional('action', {
52+
switch: [
53+
{ is: 'info', then: Joi.string() },
54+
{ is: 'create', then: Joi.object().required() },
55+
],
56+
otherwise: Joi.forbidden(),
57+
});
58+
`,
59+
filename: join(dirname, 'filename.ts'),
60+
},
61+
{
62+
// Joi with CommonJS require
63+
code: `
64+
const Joi = require('joi');
65+
const schema = Joi.string().when('field', {
66+
is: 'value',
67+
then: Joi.string().required(),
68+
});
69+
`,
70+
filename: join(dirname, 'filename.js'),
71+
},
3672
],
3773
invalid: [
3874
{

packages/jsts/src/rules/S7739/no-validation-lib/unit.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,19 @@ describe('S7739', () => {
128128
`,
129129
filename: testFilePath,
130130
},
131+
// False Positive Pattern 5d: Assignment expression to Promise
132+
{
133+
code: `
134+
let Promise;
135+
Promise = function(executor) {
136+
this.state = 'pending';
137+
this.then = function(onFulfilled) {
138+
this.onFulfilled = onFulfilled;
139+
};
140+
};
141+
`,
142+
filename: testFilePath,
143+
},
131144
// False Positive Pattern 5c: Arrow function assigned to Deferred
132145
{
133146
code: `
@@ -238,6 +251,73 @@ describe('S7739', () => {
238251
filename: testFilePath,
239252
errors: [{ messageId: NO_THENABLE_OBJECT_ERROR }],
240253
},
254+
// True Positive: Property with computed key - 'then' is still flagged
255+
{
256+
code: `
257+
const obj = {
258+
[1 + 1]: 'computed',
259+
then: function() { return this; },
260+
};
261+
`,
262+
filename: testFilePath,
263+
errors: [{ messageId: NO_THENABLE_OBJECT_ERROR }],
264+
},
265+
// True Positive: Arrow function assignment NOT delegating to .then()
266+
{
267+
code: `
268+
const result = {};
269+
result.then = (args) => someOtherCall(args);
270+
`,
271+
filename: testFilePath,
272+
errors: [{ messageId: NO_THENABLE_OBJECT_ERROR }],
273+
},
274+
// {is, then} pattern should be flagged without validation library dependency
275+
{
276+
code: `
277+
const internals = {
278+
when(field, options) { return options; },
279+
};
280+
const schema = internals.when('leftOperand', {
281+
is: 'someValue',
282+
then: { type: 'string' },
283+
otherwise: { type: 'number' },
284+
});
285+
`,
286+
filename: testFilePath,
287+
errors: [{ messageId: NO_THENABLE_OBJECT_ERROR }],
288+
},
289+
{
290+
code: `
291+
const switchCases = [
292+
{
293+
is: 'info',
294+
then: { type: 'allow' },
295+
},
296+
{
297+
is: 'create',
298+
then: { type: 'object', required: true },
299+
},
300+
];
301+
`,
302+
filename: testFilePath,
303+
errors: 2,
304+
},
305+
{
306+
code: `
307+
function createValidator() {
308+
return {
309+
when(field, options) { return this; },
310+
required() { return this; },
311+
};
312+
}
313+
const nameSchema = createValidator().when('hasName', {
314+
is: true,
315+
then: (schema) => schema,
316+
});
317+
`,
318+
filename: testFilePath,
319+
errors: [{ messageId: NO_THENABLE_OBJECT_ERROR }],
320+
},
241321
],
242322
});
243323
});

packages/jsts/src/rules/S7739/rule.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
interceptReport,
2626
isIdentifier,
2727
} from '../helpers/index.js';
28+
import { getDependenciesSanitizePaths } from '../helpers/package-jsons/dependencies.js';
2829
import * as meta from './generated-meta.js';
2930

3031
const noThenable = rules['no-thenable'];
@@ -37,9 +38,17 @@ const noThenable = rules['no-thenable'];
3738
const EXCEPTION_LIBRARIES = ['yup', 'joi'];
3839

3940
/**
40-
* Checks if a node is inside a call expression from one of the exception libraries
41+
* Checks if a node is inside a call expression from one of the exception libraries.
42+
* Uses two detection strategies:
43+
* 1. FQN check on ancestor CallExpressions (handles direct calls like yup.object({then: ...}))
44+
* 2. Conditional validation config pattern ({is, then}) when a validation library is a dependency
4145
*/
4246
function isInsideExceptionLibraryCall(context: Rule.RuleContext, node: Node): boolean {
47+
const dependencies = getDependenciesSanitizePaths(context);
48+
if (!EXCEPTION_LIBRARIES.some(lib => dependencies.has(lib))) {
49+
return false;
50+
}
51+
4352
const ancestors = context.sourceCode.getAncestors(node);
4453

4554
for (const ancestor of ancestors) {
@@ -51,7 +60,7 @@ function isInsideExceptionLibraryCall(context: Rule.RuleContext, node: Node): bo
5160
}
5261
}
5362

54-
return false;
63+
return isConditionalValidationConfig(node);
5564
}
5665

5766
/**
@@ -304,6 +313,22 @@ function hasSiblingThenableMethods(node: Node): boolean {
304313
return hasCompleteThenable(propertyNames);
305314
}
306315

316+
/**
317+
* Checks if 'then' is inside an object that also has an 'is' property,
318+
* indicating a conditional validation config pattern (e.g., {is: ..., then: ...}).
319+
* This pattern is used by validation libraries like Yup and Joi in their
320+
* .when() and .conditional() methods.
321+
*/
322+
function isConditionalValidationConfig(node: Node): boolean {
323+
const ancestors = getAncestorsWithParent(node);
324+
const objectExpr = ancestors.find(a => a.type === 'ObjectExpression');
325+
if (objectExpr?.type !== 'ObjectExpression') {
326+
return false;
327+
}
328+
const propertyNames = collectPropertyNames(objectExpr);
329+
return propertyNames.has('then') && propertyNames.has('is');
330+
}
331+
307332
/**
308333
* Checks if the reported node represents an intentional thenable implementation
309334
* that should not be flagged.

packages/jsts/src/rules/S7739/yup-project/unit.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,63 @@ describe('S7739', () => {
3333
`,
3434
filename: join(dirname, 'filename.ts'),
3535
},
36+
{
37+
// Chained yup calls - string().when() with {is, then} config
38+
code: `
39+
import * as yup from 'yup';
40+
const schema = yup.string().when('hasName', {
41+
is: true,
42+
then: (schema) => schema.required(),
43+
});
44+
`,
45+
filename: join(dirname, 'filename.ts'),
46+
},
47+
{
48+
// Chained yup calls inside object shape definition
49+
// This is the most common real-world pattern from the Jira ticket
50+
code: `
51+
import * as yup from 'yup';
52+
const formSchema = yup.object().shape({
53+
age: yup.number().required(),
54+
parentalConsent: yup.bool().when('age', {
55+
is: (age) => age < 18,
56+
then: (schema) => schema.required(),
57+
otherwise: (schema) => schema.optional(),
58+
}),
59+
});
60+
`,
61+
filename: join(dirname, 'filename.ts'),
62+
},
63+
{
64+
// Multiple chained .when() calls with {is, then} in same shape
65+
code: `
66+
import * as yup from 'yup';
67+
const schema = yup.object().shape({
68+
a1: yup.string().when('a2', {
69+
is: undefined,
70+
then: (schema) => schema.required(),
71+
}),
72+
a2: yup.string().when('a1', {
73+
is: undefined,
74+
then: (schema) => schema.required(),
75+
}),
76+
});
77+
`,
78+
filename: join(dirname, 'filename.ts'),
79+
},
80+
{
81+
// Yup .when() with array of dependencies
82+
code: `
83+
import * as yup from 'yup';
84+
const schema = yup.object().shape({
85+
value: yup.number().when(['unknownDep', 'knownDep'], {
86+
is: true,
87+
then: (s) => s.required(),
88+
}),
89+
});
90+
`,
91+
filename: join(dirname, 'filename.ts'),
92+
},
3693
],
3794
invalid: [
3895
{

0 commit comments

Comments
 (0)