Skip to content

Commit

Permalink
Merge pull request #1211 from bmish/require-tagless-components-false-…
Browse files Browse the repository at this point in the history
…positive-service

Fix false positive with non-components in `require-tagless-components` rule
  • Loading branch information
bmish authored May 29, 2021
2 parents 4a70986 + 4960bb8 commit 10ac49f
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 4 deletions.
26 changes: 22 additions & 4 deletions lib/rules/require-tagless-components.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
'use strict';

const { isEmberComponent } = require('../utils/ember');
const { getSourceModuleNameForIdentifier } = require('../utils/import');
const {
isCallExpression,
isIdentifier,
isObjectExpression,
isStringLiteral,
} = require('../utils/types');
const { getNodeOrNodeFromVariable } = require('../utils/utils');
const { getImportIdentifier } = require('../utils/import');
const { isTestFile } = require('../utils/ember');

const ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS =
"Please switch to a tagless component by setting `tagName: ''` or converting to a Glimmer component";
Expand Down Expand Up @@ -101,15 +101,30 @@ module.exports = {
},

create(context) {
if (isTestFile(context.getFilename())) {
// This rule does not apply to test files.
return {};
}

const sourceCode = context.getSourceCode();
const { scopeManager } = sourceCode;

let importedComponentName;

return {
ImportDeclaration(node) {
if (node.source.value === '@ember/component') {
importedComponentName =
importedComponentName || getImportIdentifier(node, '@ember/component');
}
},

// Handle classic components
'CallExpression > MemberExpression[property.name="extend"]'(node) {
const callExpression = node.parent;

if (!isEmberComponent(context, callExpression)) {
if (!(node.object.type === 'Identifier' && node.object.name === importedComponentName)) {
// Not an Ember component.
return;
}

Expand Down Expand Up @@ -138,7 +153,10 @@ module.exports = {

// Handle ES class components
'ClassDeclaration[superClass]'(node) {
if (getSourceModuleNameForIdentifier(context, node.superClass) === '@ember/component') {
if (
node.superClass.type === 'Identifier' &&
node.superClass.name === importedComponentName
) {
let tagNameNode;
let decorator;

Expand Down
35 changes: 35 additions & 0 deletions tests/lib/rules/require-tagless-components.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,22 @@ ruleTester.run('require-tagless-components', rule, {
import Component from '@ember/component';
export default Component.extend({ tagName: '' });
`,
`
import Component from '@ember/component';
export default Component.extend(Mixin, { tagName: '' });
`,
`
import Component from '@ember/component';
export default class MyComponent extends Component {
tagName = ''
}
`,
`
import Component from '@ember/component';
export default class MyComponent extends Component.extend(Mixin) {
tagName = ''
}
`,
`
import Component from '@ember/component';
import { tagName } from '@ember-decorators/component';
Expand All @@ -56,6 +66,31 @@ ruleTester.run('require-tagless-components', rule, {
tagName = 'some-non-empty-value';
}
`,
{
// Classic service in component file.
code: `
import Service from '@ember/service';
export default Service.extend({});
`,
filename: 'app/components/foo.js',
},
{
// Native service in component file.
code: `
import Service from '@ember/service';
export default class MyService extends Service {};
`,
filename: 'app/components/foo.js',
},
{
// Should ignore test files.
code: `
import Component from '@ember/component';
export default class MyComponent extends Component {};
Component.extend({});
`,
filename: 'tests/integration/components/foo-test.js',
},
],
invalid: [
{
Expand Down

0 comments on commit 10ac49f

Please sign in to comment.