Repository: ember-cli/eslint-plugin-ember (js)

Issue:
`no-classic-classes`: `getSourceModuleName` should only be called on a `CallExpression`, `MemberExpression` or `Identifier`
```js
AssertionError [ERR_ASSERTION]: `getSourceModuleName` should only be called on a `CallExpression`, `MemberExpression` or `Identifier`
    at getSourceModuleName (/private/tmp/test-app/node_modules/eslint-plugin-ember/lib/utils/import.js:46:5)
    at /private/tmp/test-app/node_modules/eslint-plugin-ember/lib/utils/import.js:31:49
    at Array.some (<anonymous>)
    at /private/tmp/test-app/node_modules/eslint-plugin-ember/lib/utils/import.js:30:36
    at Array.find (<anonymous>)
    at getSourceModuleNameForIdentifier (/private/tmp/test-app/node_modules/eslint-plugin-ember/lib/utils/import.js:29:6)
    at CallExpression > MemberExpression[property.name="extend"] (/private/tmp/test-app/node_modules/eslint-plugin-ember/lib/rules/no-classic-classes.js:63:37)
    at /private/tmp/test-app/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/private/tmp/test-app/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
```

is triggered by:

```js
import thing from 'place';

export default class Cls {
  init() {
    this.extend();
  }
}
```

(without the import statement, even though it's unused, the error will not happen).

* `eslint@7.20.0`
* `eslint-plugin-ember@10.2.0`
* `ember-source@3.25.1`
* `ember-cli@3.25.0`

Looks similar to #905, but it was fixed by #906 which was released in v8.10.1 and I'm seeing this in 10.2.0.

Pull request:
Fix crash with `this.extend()` in `no-classic-classes` rule
Fixes #1088.

TODO:
- [x] Handle `import thing from 'place'; this.foo.bar.extend();`

Patch:
diff --git a/lib/rules/no-classic-classes.js b/lib/rules/no-classic-classes.js
index 3dff2a34..202cc229 100644
--- 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
index 1251323b..cb7909cd 100644
--- 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;
+}


Test patch:
diff --git a/tests/lib/rules/no-classic-classes.js b/tests/lib/rules/no-classic-classes.js
index 7127dfd2..5c9c0467 100644
--- 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
index 8d9ef0a5..d86b9493 100644
--- a/tests/lib/utils/utils-test.js
+++ b/tests/lib/utils/utils-test.js
@@ -194,3 +194,19 @@ 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();
+  });
+});