Check imports when detecting computed properties in many rules

[bmish]

Updates our Ember util function isComputedProp to consider the names that Ember and computed are imported under instead of assuming they are imported under the standard names. This util function is used in 10 rules for detecting computed properties.

CC: @mongoose700

Helps with #590.

[mongoose700]

These should be computed. rather than computed().

[bmish]

Oh actually I will change this comment to say:

// Ember.computed().volatile() or computed().volatile()

to avoid confusion with the macro.

[mongoose700]

Ah. It could also help to do Ember.computed(...).volatile(), since it isn't valid without a function as an argument to the computed call.

[mongoose700]

Could you add comments explaining these arguments?

[bmish]

Good idea, added jsdoc comments.

[mongoose700]

We could extract this set outside of the function, so that don't have to keep creating it.

[bmish]

Fixed.

[mongoose700]

This logic is getting repeated a lot of times. It could be nice to make a more generic imports object, and do something like imports = updateImports(imports, node). Then isComputedProp could take that imports object as an argument.

[rwjblue]

Ya, or even something like:

let imports = new Map();

return {
  ImportDeclaration: gatherImports(imports),

  // ...snip...
}

Where the gatherImports method returns a function that works as a visitor handler itself, or if you also have custom import logic you need you could do this (with the same API):

let imports = new Map();
let importGatherer = gatherImports(imports);

return {
  ImportDeclaration(node) {
    importGatherer(node);
    // other logic
  }
};

[bmish]

Definitely agree with this. I may do this as a follow-up so I can focus on this improved import gathering API by itself.

[bmish]

Working on the new API in this PR: #910

[rwjblue]

FWIW, there is nothing wrong with this (from a technical level):

import Ember from 'ember';
import Em from 'ember';
import LOL from 'ember;

All in the same module, and IMHO ^ should still trigger the rule. If I understand this code correctly, we are assuming there is only one.

[mongoose700]

While dealing with this would be a large hassle to just tack on to the current implementation, I think it would be pretty easy to handle with the gatherImports-style refactor.

[bmish]

Will consider this in the future, although it does seem like it could be pretty inconvenient to handle this edge case. Hopefully, most people are using a rule like import/no-duplicates to avoid this.

[rwjblue]

Maybe, not sure. I would expect that the goal of the a higher level gatherImports style API is to abstract this issue away.