Skip to content

Commit aa7b664

Browse files
bmishbmish
authored
Merge pull request #892 from bmish/no-get-static-error-message
Switch from unnecessary dynamic error message to static error message in `no-get` rule
2 parents 2a001c4 + 18538ca commit aa7b664

File tree

2 files changed

+24
-65
lines changed

2 files changed

+24
-65
lines changed

‎lib/rules/no-get.js

Lines changed: 4 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,7 @@ const utils = require('../utils/utils');
66
const assert = require('assert');
77
const { getImportIdentifier } = require('../utils/import');
88

9-
function makeErrorMessageForGet(
10-
property,
11-
{ isImportedGet, useOptionalChaining, isInLeftSideOfAssignmentExpression } = {}
12-
) {
13-
const original = isImportedGet ? `get(this, '${property}')` : `this.get('${property}')`;
14-
15-
const replacements = [
16-
`this.${property}`,
17-
useOptionalChaining && !isInLeftSideOfAssignmentExpression
18-
? `this.${property.replace(/\./g, '?.')}`
19-
: undefined,
20-
]
21-
.filter(Boolean)
22-
.join('` or `');
23-
24-
return `Use \`${replacements}\` instead of \`${original}\``;
25-
}
9+
const ERROR_MESSAGE_GET = "Use ES5 getters (`this.property`) instead of Ember's `get` function";
2610

2711
const ERROR_MESSAGE_GET_PROPERTIES =
2812
"Use `{ prop1: this.prop1, prop2: this.prop2, ... }` instead of Ember's `getProperties` function";
@@ -35,15 +19,11 @@ function isValidJSPath(str) {
3519
return str.split('.').every(isValidJSVariableName);
3620
}
3721

38-
function reportGet({ node, context, path, isImportedGet, useOptionalChaining }) {
22+
function reportGet({ node, context, path, useOptionalChaining }) {
3923
const isInLeftSideOfAssignmentExpression = utils.isInLeftSideOfAssignmentExpression(node);
4024
context.report({
4125
node,
42-
message: makeErrorMessageForGet(path, {
43-
isImportedGet,
44-
useOptionalChaining,
45-
isInLeftSideOfAssignmentExpression,
46-
}),
26+
message: ERROR_MESSAGE_GET,
4727
fix(fixer) {
4828
return fixGet({ node, fixer, path, useOptionalChaining, isInLeftSideOfAssignmentExpression });
4929
},
@@ -68,7 +48,7 @@ function fixGet({ node, fixer, path, useOptionalChaining, isInLeftSideOfAssignme
6848
}
6949

7050
module.exports = {
71-
makeErrorMessageForGet,
51+
ERROR_MESSAGE_GET,
7252
ERROR_MESSAGE_GET_PROPERTIES,
7353
meta: {
7454
type: 'suggestion',

‎tests/lib/rules/no-get.js

Lines changed: 20 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const path = require('path');
22
const rule = require('../../../lib/rules/no-get');
33
const RuleTester = require('eslint').RuleTester;
44

5-
const { makeErrorMessageForGet, ERROR_MESSAGE_GET_PROPERTIES } = rule;
5+
const { ERROR_MESSAGE_GET, ERROR_MESSAGE_GET_PROPERTIES } = rule;
66

77
const ruleTester = new RuleTester({
88
parser: require.resolve('babel-eslint'),
@@ -186,8 +186,7 @@ ruleTester.run('no-get', rule, {
186186
{
187187
code: "this.get('foo');",
188188
output: 'this.foo;',
189-
// Error message intentionally written out to ensure it looks right.
190-
errors: [{ message: "Use `this.foo` instead of `this.get('foo')`", type: 'CallExpression' }],
189+
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
191190
},
192191
{
193192
code: `
@@ -202,16 +201,15 @@ ruleTester.run('no-get', rule, {
202201
import { random } from 'random';
203202
this.foo;
204203
`,
205-
// Error message intentionally written out to ensure it looks right.
206-
errors: [{ message: "Use `this.foo` instead of `get(this, 'foo')`", type: 'CallExpression' }],
204+
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
207205
},
208206
{
209207
// With renamed import:
210208
code: "import { get as g } from '@ember/object'; g(this, 'foo');",
211209
output: "import { get as g } from '@ember/object'; this.foo;",
212210
errors: [
213211
{
214-
message: makeErrorMessageForGet('foo', { isImportedGet: true }),
212+
message: ERROR_MESSAGE_GET,
215213
type: 'CallExpression',
216214
},
217215
],
@@ -221,7 +219,7 @@ ruleTester.run('no-get', rule, {
221219
output: 'this.foo.someFunction();',
222220
errors: [
223221
{
224-
message: makeErrorMessageForGet('foo', { isImportedGet: false }),
222+
message: ERROR_MESSAGE_GET,
225223
type: 'CallExpression',
226224
},
227225
],
@@ -232,7 +230,7 @@ ruleTester.run('no-get', rule, {
232230
output: null,
233231
errors: [
234232
{
235-
message: makeErrorMessageForGet('foo-bar', { isImportedGet: false }),
233+
message: ERROR_MESSAGE_GET,
236234
type: 'CallExpression',
237235
},
238236
],
@@ -243,7 +241,7 @@ ruleTester.run('no-get', rule, {
243241
output: null,
244242
errors: [
245243
{
246-
message: makeErrorMessageForGet('foo-bar', { isImportedGet: true }),
244+
message: ERROR_MESSAGE_GET,
247245
type: 'CallExpression',
248246
},
249247
],
@@ -292,7 +290,7 @@ ruleTester.run('no-get', rule, {
292290
output: null,
293291
errors: [
294292
{
295-
message: makeErrorMessageForGet('foo.bar', { isImportedGet: false }),
293+
message: ERROR_MESSAGE_GET,
296294
type: 'CallExpression',
297295
},
298296
],
@@ -302,7 +300,7 @@ ruleTester.run('no-get', rule, {
302300
output: null,
303301
errors: [
304302
{
305-
message: makeErrorMessageForGet('foo.bar', { isImportedGet: true }),
303+
message: ERROR_MESSAGE_GET,
306304
type: 'CallExpression',
307305
},
308306
],
@@ -325,8 +323,7 @@ ruleTester.run('no-get', rule, {
325323
output: 'this.foo?.bar;',
326324
errors: [
327325
{
328-
// Error message intentionally written out to ensure it looks right.
329-
message: "Use `this.foo.bar` or `this.foo?.bar` instead of `this.get('foo.bar')`",
326+
message: ERROR_MESSAGE_GET,
330327
type: 'CallExpression',
331328
},
332329
],
@@ -337,10 +334,7 @@ ruleTester.run('no-get', rule, {
337334
output: 'this.very?.long?.path;',
338335
errors: [
339336
{
340-
message: makeErrorMessageForGet('very.long.path', {
341-
isImportedGet: false,
342-
useOptionalChaining: true,
343-
}),
337+
message: ERROR_MESSAGE_GET,
344338
type: 'CallExpression',
345339
},
346340
],
@@ -351,10 +345,7 @@ ruleTester.run('no-get', rule, {
351345
output: "import { get } from '@ember/object'; this.foo?.bar;",
352346
errors: [
353347
{
354-
message: makeErrorMessageForGet('foo.bar', {
355-
isImportedGet: true,
356-
useOptionalChaining: true,
357-
}),
348+
message: ERROR_MESSAGE_GET,
358349
type: 'CallExpression',
359350
},
360351
],
@@ -365,10 +356,7 @@ ruleTester.run('no-get', rule, {
365356
output: "import { get } from '@ember/object'; this.very?.long?.path;",
366357
errors: [
367358
{
368-
message: makeErrorMessageForGet('very.long.path', {
369-
isImportedGet: true,
370-
useOptionalChaining: true,
371-
}),
359+
message: ERROR_MESSAGE_GET,
372360
type: 'CallExpression',
373361
},
374362
],
@@ -379,10 +367,7 @@ ruleTester.run('no-get', rule, {
379367
output: 'this.foo;',
380368
errors: [
381369
{
382-
message: makeErrorMessageForGet('foo', {
383-
isImportedGet: false,
384-
useOptionalChaining: true,
385-
}),
370+
message: ERROR_MESSAGE_GET,
386371
type: 'CallExpression',
387372
},
388373
],
@@ -395,10 +380,7 @@ ruleTester.run('no-get', rule, {
395380
output: "this.foo.bar[123] = 'hello world';",
396381
errors: [
397382
{
398-
message: makeErrorMessageForGet('foo.bar', {
399-
isImportedGet: false,
400-
useOptionalChaining: false,
401-
}),
383+
message: ERROR_MESSAGE_GET,
402384
type: 'CallExpression',
403385
},
404386
],
@@ -411,10 +393,7 @@ ruleTester.run('no-get', rule, {
411393
output: "this.foo.bar[123] = 'hello world';",
412394
errors: [
413395
{
414-
message: makeErrorMessageForGet('foo.bar', {
415-
isImportedGet: false,
416-
useOptionalChaining: false,
417-
}),
396+
message: ERROR_MESSAGE_GET,
418397
type: 'CallExpression',
419398
},
420399
],
@@ -442,7 +421,7 @@ ruleTester.run('no-get', rule, {
442421
});
443422
this.propertyOutsideClass;
444423
`,
445-
errors: [{ message: makeErrorMessageForGet('propertyOutsideClass'), type: 'CallExpression' }],
424+
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
446425
},
447426
{
448427
// Reports violation after (native) proxy class.
@@ -466,7 +445,7 @@ ruleTester.run('no-get', rule, {
466445
}
467446
this.propertyOutsideClass;
468447
`,
469-
errors: [{ message: makeErrorMessageForGet('propertyOutsideClass'), type: 'CallExpression' }],
448+
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
470449
},
471450

472451
{
@@ -491,7 +470,7 @@ ruleTester.run('no-get', rule, {
491470
});
492471
this.propertyOutsideClass;
493472
`,
494-
errors: [{ message: makeErrorMessageForGet('propertyOutsideClass'), type: 'CallExpression' }],
473+
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
495474
},
496475
{
497476
// Reports violation after (native) class with `unknownProperty()`.
@@ -515,7 +494,7 @@ ruleTester.run('no-get', rule, {
515494
}
516495
this.propertyOutsideClass;
517496
`,
518-
errors: [{ message: makeErrorMessageForGet('propertyOutsideClass'), type: 'CallExpression' }],
497+
errors: [{ message: ERROR_MESSAGE_GET, type: 'CallExpression' }],
519498
},
520499
],
521500
});

0 commit comments

Comments
 (0)