Skip to content

Commit

Permalink
Merge branch 'main' into no-unused-placeholders-report-loc
Browse files Browse the repository at this point in the history
* main:
  fix: reporting location in no-missing-placeholders (eslint-community#280)
  fix: clarify report messages for no-missing-placeholders and no-unused-placeholders (eslint-community#278)
  fix: allow additional schema types in require-meta-schema (eslint-community#277)
  • Loading branch information
bmish committed Aug 4, 2022
2 parents 03c53c6 + 31ff45c commit 0cfb471
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 21 deletions.
12 changes: 7 additions & 5 deletions lib/rules/no-missing-placeholders.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module.exports = {
schema: [],
messages: {
placeholderDoesNotExist:
'The placeholder {{{{missingKey}}}} does not exist.',
"The placeholder {{{{missingKey}}}} is missing (must provide it in the report's `data` object).",
},
},

Expand Down Expand Up @@ -80,9 +80,11 @@ module.exports = {
});
}

for (const { message, data } of reportMessagesAndDataArray.filter(
(obj) => obj.message
)) {
for (const {
message,
messageId,
data,
} of reportMessagesAndDataArray.filter((obj) => obj.message)) {
const messageStaticValue = getStaticValue(
message,
context.getScope()
Expand Down Expand Up @@ -112,7 +114,7 @@ module.exports = {

if (!matchingProperty) {
context.report({
node: message,
node: data || messageId || message,
messageId: 'placeholderDoesNotExist',
data: { missingKey: match[1] },
});
Expand Down
3 changes: 2 additions & 1 deletion lib/rules/no-unused-placeholders.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ module.exports = {
fixable: null,
schema: [],
messages: {
placeholderUnused: 'The placeholder {{{{unusedKey}}}} is unused.',
placeholderUnused:
'The placeholder {{{{unusedKey}}}} is unused (does not exist in the actual message).',
},
},

Expand Down
5 changes: 4 additions & 1 deletion lib/rules/require-meta-schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ module.exports = {
hasEmptySchema = true;
}

if (!['ArrayExpression', 'ObjectExpression'].includes(value.type)) {
if (
value.type === 'Literal' ||
(value.type === 'Identifier' && value.name === 'undefined')
) {
context.report({ node: value, messageId: 'wrongType' });
}
},
Expand Down
54 changes: 41 additions & 13 deletions tests/lib/rules/no-missing-placeholders.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ const RuleTester = require('eslint').RuleTester;
* @param {string} missingKey The placeholder that is missing
* @returns {object} An expected error
*/
function error(missingKey, type = 'Literal') {
return { type, message: `The placeholder {{${missingKey}}} does not exist.` };
function error(missingKey, type, extra) {
return {
type,
message: `The placeholder {{${missingKey}}} is missing (must provide it in the report's \`data\` object).`,
...extra,
};
}

// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -232,7 +236,7 @@ ruleTester.run('no-missing-placeholders', rule, {
}
};
`,
errors: [error('bar')],
errors: [error('bar', 'Literal')],
},
{
code: `
Expand All @@ -246,7 +250,7 @@ ruleTester.run('no-missing-placeholders', rule, {
}
};
`,
errors: [error('bar')],
errors: [error('bar', 'ObjectExpression')],
},
{
code: `
Expand All @@ -260,15 +264,15 @@ ruleTester.run('no-missing-placeholders', rule, {
}
};
`,
errors: [error('hasOwnProperty')],
errors: [error('hasOwnProperty', 'ObjectExpression')],
},
{
code: `
module.exports = context => {
context.report(node, 'foo {{bar}}', { baz: 'qux' }); return {};
};
`,
errors: [error('bar')],
errors: [error('bar', 'ObjectExpression')],
},
{
// Message in variable.
Expand All @@ -278,15 +282,15 @@ ruleTester.run('no-missing-placeholders', rule, {
context.report(node, MESSAGE, { baz: 'qux' }); return {};
};
`,
errors: [error('bar', 'Identifier')],
errors: [error('bar', 'ObjectExpression')],
},
{
code: `
module.exports = context => {
context.report(node, { line: 1, column: 3 }, 'foo {{bar}}', { baz: 'baz' }); return {};
};
`,
errors: [error('bar')],
errors: [error('bar', 'ObjectExpression')],
},
{
code: `
Expand All @@ -300,7 +304,19 @@ ruleTester.run('no-missing-placeholders', rule, {
}
};
`,
errors: [error('bar')],
errors: [
error(
'bar',
'ObjectExpression',
// report on data
{
line: 7,
endLine: 7,
column: 21,
endColumn: 39,
}
),
],
},

{
Expand Down Expand Up @@ -340,7 +356,7 @@ ruleTester.run('no-missing-placeholders', rule, {
}
};
`,
errors: [error('bar')],
errors: [error('bar', 'ObjectExpression')],
},
{
// Suggestion and messageId
Expand All @@ -359,7 +375,7 @@ ruleTester.run('no-missing-placeholders', rule, {
}
};
`,
errors: [error('bar')],
errors: [error('bar', 'Literal')],
},
{
// `create` in variable.
Expand All @@ -373,7 +389,7 @@ ruleTester.run('no-missing-placeholders', rule, {
}
module.exports = { create };
`,
errors: [error('hasOwnProperty')],
errors: [error('hasOwnProperty', 'ObjectExpression')],
},
{
// messageId.
Expand All @@ -388,7 +404,19 @@ ruleTester.run('no-missing-placeholders', rule, {
}
};
`,
errors: [error('bar')],
errors: [
error(
'bar',
'Literal',
// report on the messageId
{
line: 7,
endLine: 7,
column: 26,
endColumn: 39,
}
),
],
},
],
});
2 changes: 1 addition & 1 deletion tests/lib/rules/no-unused-placeholders.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const RuleTester = require('eslint').RuleTester;
function error(unusedKey, extra) {
return {
type: 'Property', // The property in the report's `data` object for the unused placeholder.
message: `The placeholder {{${unusedKey}}} is unused.`,
message: `The placeholder {{${unusedKey}}} is unused (does not exist in the actual message).`,
...extra,
};
}
Expand Down
59 changes: 59 additions & 0 deletions tests/lib/rules/require-meta-schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,57 @@ ruleTester.run('require-meta-schema', rule, {
`,
parserOptions: { sourceType: 'module' },
},
// Variable schema with array value.
`
const schema = [];
module.exports = {
meta: { schema },
create(context) {}
};
`,
// Variable schema with object value.
`
const foo = {};
module.exports = {
meta: { schema: foo },
create(context) {}
};
`,
// Variable schema with no static value.
`
module.exports = {
meta: { schema },
create(context) {}
};
`,
// Variable schema pointing to unknown variable chain.
`
module.exports = {
meta: { schema: baseRule.meta.schema },
create(context) {}
};
`,
// Schema with function call as value.
`
module.exports = {
meta: { schema: getSchema() },
create(context) {}
};
`,
// Schema with ternary (conditional) expression.
`
module.exports = {
meta: { schema: foo ? [] : {} },
create(context) {}
};
`,
// Schema with logical expression.
`
module.exports = {
meta: { schema: foo || {} },
create(context) {}
};
`,
`
let schema;
schema = foo ? [] : {};
Expand Down Expand Up @@ -296,6 +333,28 @@ schema: [] },
output: null,
errors: [{ messageId: 'wrongType', type: 'Identifier', suggestions: [] }],
},
{
// Schema with number literal value.
code: `
module.exports = {
meta: { schema: 123 },
create(context) {}
};
`,
output: null,
errors: [{ messageId: 'wrongType', type: 'Literal', suggestions: [] }],
},
{
// Schema with string literal value.
code: `
module.exports = {
meta: { schema: 'hello world' },
create(context) {}
};
`,
output: null,
errors: [{ messageId: 'wrongType', type: 'Literal', suggestions: [] }],
},
{
code: `
const schema = null;
Expand Down

0 comments on commit 0cfb471

Please sign in to comment.