fix: crash with this.extend() in no-classic-classes rule

ember-cli · Mar 22, 2021 · 005d045 · 005d045
1 parent 7a65db3
commit 005d045
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 1 deletion.
diff --git a/lib/rules/no-classic-classes.js b/lib/rules/no-classic-classes.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const { getSourceModuleNameForIdentifier } = require('../utils/import');
+const { startsWithThisExpression } = require('../utils/utils');
 const { isObjectExpression } = require('../utils/types');
 
 const ERROR_MESSAGE_NO_CLASSIC_CLASSES =
@@ -77,7 +78,11 @@ module.exports = {
         // Invalid if there are no arguments because that is equivalent to not called `extend` at all
         // Invalid with an object as an argument because that logic needs conversion to a native class
         // Still allows `.extend` if some other identifier is passed, like a Mixin
-        if (hasNoArguments(callExpression) || hasObjectArgument(callExpression)) {
+        if (
+          (hasNoArguments(callExpression) || hasObjectArgument(callExpression)) &&
+          ['Identifier', 'MemberExpression'].includes(node.object.type) &&
+          !startsWithThisExpression(node)
+        ) {
           const classImportedFrom = getSourceModuleNameForIdentifier(context, node.object);
           if (
             isEmberImport(classImportedFrom) ||

diff --git a/lib/utils/utils.js b/lib/utils/utils.js
@@ -10,6 +10,7 @@ const {
   isObjectPattern,
   isOptionalCallExpression,
   isOptionalMemberExpression,
+  isThisExpression,
 } = require('../utils/types');
 
 module.exports = {
@@ -24,6 +25,7 @@ module.exports = {
   isInLeftSideOfAssignmentExpression,
   parseArgs,
   parseCallee,
+  startsWithThisExpression,
 };
 
 /**
@@ -250,3 +252,16 @@ function isInLeftSideOfAssignmentExpression(node) {
     )
   );
 }
+
+function startsWithThisExpression(node) {
+  if (isCallExpression(node) && node.callee) {
+    return startsWithThisExpression(node.callee);
+  } else if (isMemberExpression(node) && node.object) {
+    return startsWithThisExpression(node.object);
+  } else if (isIdentifier(node)) {
+    return false;
+  } else if (isThisExpression(node)) {
+    return true;
+  }
+  return false;
+}
diff --git a/tests/lib/rules/no-classic-classes.js b/tests/lib/rules/no-classic-classes.js
@@ -35,6 +35,10 @@ ruleTester.run('no-classic-classes', rule, {
       export default SomeOtherThing.extend({});
     `,
     'export default Component.extend({});', // No import
+
+    'this.extend();',
+    'this.foo.extend();',
+    'this.foo.bar.extend();',
   ],
 
   invalid: [

diff --git a/tests/lib/utils/utils-test.js b/tests/lib/utils/utils-test.js
@@ -194,3 +194,18 @@ describe('parseCallee', () => {
     expect(parsedCallee).toStrictEqual(['Ember', 'A']);
   });
 });
+
+describe('startsWithThisExpression', () => {
+  it('should behave correctly', () => {
+    expect(utils.startsWithThisExpression(parse('x'))).toBeFalsy();
+    expect(utils.startsWithThisExpression(parse('x.y'))).toBeFalsy();
+    expect(utils.startsWithThisExpression(parse('x()'))).toBeFalsy();
+    expect(utils.startsWithThisExpression(parse('x.y()'))).toBeFalsy();
+    expect(utils.startsWithThisExpression(parse('x.y.z()'))).toBeFalsy();
+    expect(utils.startsWithThisExpression(parse('this'))).toBeTruthy();
+    expect(utils.startsWithThisExpression(parse('this.x'))).toBeTruthy();
+    expect(utils.startsWithThisExpression(parse('this.x.y'))).toBeTruthy();
+    expect(utils.startsWithThisExpression(parse('this.x()'))).toBeTruthy();
+    expect(utils.startsWithThisExpression(parse('this.x.y()'))).toBeTruthy();
+  });
+});