Import no-attrs-snapshot from @chadhietala's https://github.com/ember-best-practices/eslint-plugin-ember-best-practices/blob/master/guides/rules/no-attrs-snapshot.md

I have an application running ember 2.6.2 and after installing eslint-plugin-ember I'm met with the following:

ember s --proxy
An addon must define a `name` property.

Is there anything I'm missing here? I took a brief look at the dependencies for the addon and nothing jumped out at me.

Many thanks.

When using https://github.com/rwjblue/ember-computed-decorators, you'll get lint errors because multiline computed properties are now seen as methods and according to the order-in-controllers rule for example, should go below actions.

The various component level rules do not get applied if you extend another application level component.

For example this shows no linter errors when it should:

export default MyComponent.extend({
  foo: 'bar',
   store: inject.service()
});

Whereas this does:

export default Component.extend({
  foo: 'bar',
   store: inject.service()
});

see https://github.com/chadhietala/ember-best-practices

According to https://github.com/ember-cli/ember-rfc176-data I think we can add a rule that will check if reserved identifiers like Object are being imported.

Incorrect:

import Object from '@ember/object';

Correct:

import EmberObject from '@ember/object';

All reserved keys are here: https://github.com/ember-cli/ember-rfc176-data/blob/master/reserved.json

This plugin should have its own ESLint setup present, to assure code aesthetics/style consistency.

• ESLint config, required dependencies
• Running ESLint check as part of tests on TravisCI

For example, if you are declaring a GET route in Ember CLI Mirage:

// Throws `use-ember-get-and-set` linting error
this.get('/foo', (db) => {
server.get('/foo', (db) => {

Or using this.set in an integration test:

this.set('person', { name: 'Foo' });

I just started using eslint-plugin-ember. One thing I noticed is that i get a lot of alias-model-in-controller errors. The rule makes sense, but some times we have routes where we're not interested in the model. e.g a login route or a forgot password route.
I wonder if we have any alternative. Maybe detect if the model hook was implemented or not?

Currently eslint-plugin-ember only lints standard EmberObject model classes, i.e. Controller.extend({ ... }). But native ES6 classes are on the horizon and may already be used with some caveats.

This code runs without any problems, with the necessary Babel configuration:

// Please excuse the German names here. I just copied this from our production app.
import Controller from '@ember/controller';
import { computed } from 'ember-decorators/object';
import { reads } from 'ember-decorators/object/computed';

export default class extends Controller {
  queryParams = {
    posteingangTab: {
      scope: 'controller'
    }
  };

  @reads('model') posteingang;

  @computed('posteingang')
  rezeptkopfdatenIds(posteingang) {
    return posteingang.hasMany('rezeptkopfdaten').ids();
  }
}

It would be awesome if this plugin could support both the old and new syntax.

_{Also a new rule proposal: use-es6-classes 😊}

Import "no-attrs" rule from @chadhietala's https://github.com/ember-best-practices/eslint-plugin-ember-best-practices/blob/master/guides/rules/no-attrs.md

Hi, I'm currently moving our codebase to conform to your eslint rules and am writing some codemods that people may find useful.

So far I've adapted Tom Dale's module codemod to handle your local-modules and use-ember-get-and-set rules. I've also created one for your no-on-calls-in-components rule and one for your use-brace-expansion rule.

Any feedback is welcome and let me know if the codemod doesn't align to the rule!

I get eslint error order-in-routes on component, model and mirage with name contain route:

app/components/g-map-route.js
app/models/route.js
mirage/factories/route.js

We have a local variable named attr and the no-empty-attrs rule is being triggered. Renaming the variable works but should not raise this.

someArrayOfStrings.filter(function(attr) {
  return attr.underscore().endsWith('_id');
});

@netguru
I have a readonly computed property

companies: computed(function () {
   return this.get('store').findAll('company')
 }).readOnly(),

and it says that actions: {} should be above companies

According to ember-cli's site, the node version supported is the latest LTS, which happens to be Node 6. This probably means that we can drop support for Node 4.

Consider this code in model unit test:

test('user relationship', function(assert) {
  const contact = this.store().modelFor('contact');
  const relationship = get(contact, 'relationshipsByName').get('user');
  assert.equal(relationship.key, 'user');
  assert.equal(relationship.kind, 'belongsTo');
});

This will throw an error because we're not using Ember.get getter for the user property. But we can't do it because relationshipsByName is an Ember.Map which has different get function.

My first idea was to add some optional settings to this rule where we would define particular preceding properties which shouldn't throw an error.

I'm almost sure that this won't be the only case, especially when we're using some JQuery addons which can also have their own getters (for example pickadate.js).

WDYT?

A default settings for this rule should exist in:
https://github.com/netguru/eslint-plugin-ember/blob/master/config/base.js

• Point-out oldest supported Node.js version to run rules present in this plugin in README file (likely to be v4 LTS, as of this writing).
• Update TravisCI testing matrix to run tests under it (in addition to currently used v6).

With the following...

foo: computed(function() {

}).volatile(),

bar: computed(function() {

})

I get the 'bar' multi-line function should be above 'foo' method, but I wasn't expecting to.

Component.extend({
  layout: hbs`Hello World`,
  init() {},
  actions: {}
});

layout is being detected as an unknown type, and so I am getting suggestions it should be placed lower down.

somePromise().then(this.someMethod)

will result in a named-functions-in-promises linting error, despite using a named function.

Import "no-global-jquery" rule from @chadhietala's https://github.com/ember-best-practices/eslint-plugin-ember-best-practices/blob/master/guides/rules/no-global-jquery.md

The new modules have you import Ember.inject.service like import { inject } from '@ember/service'. foo: inject() is not detected as a service by the ordering linters, so you are told to rearrange them.

It seems that the folks at LinkedIn started yet another ESLint plugin for Ember at https://github.com/ember-best-practices/eslint-plugin-ember-best-practices

IMHO we should focus our efforts on one plugin and eslint-plugin-ember seems to be the best fit for that.

I would like to propose to

1. import the rules from the other project
2. move this project into an independent GitHub org so that the people working on the other project can also act as maintainers here

/cc @michalsnik @trentmwillis @scalvert @rwjblue

Instead of accessing Ember.$ and Ember.RSVP we should:

import $ from 'jquery';
import RSVP from 'rsvp';

I think it might be a good idea to encode that in an ESLint rule

I get errors from ember/use-brace-expansion with something like this:

  sampleProperty: computed(
    'foo.bar.{name,place}',
    'foo.baz.{thing,@each.stuff}',
    'foo.qux.[]',
    function() {

It doesn't pass until I nest them all like this:

  sampleProperty: computed(
    'foo.{bar.{name,place},baz.{thing,@each.stuff},qux.[]}',
    function() {

This does not work. Ember does not support nested brace expansion, per this comment/code.

This is what I got, even if I just run ember -v:

An addon must define a `name` property.
An error occured in the constructor for eslint-plugin-ember at /Users/username/repo/node_modules/eslint-plugin-ember

alias-model-in-controller rule in documentation has an ability to be covered by setting model in route's setupController method. This is not working, and even is not intended to, so it should be removed from documentation.

Anyway, I hope there was some way to cover such scenarios. We have cases when our controller do not have model (this may be misconception on legacy code level, and can be 'not-so-easily' converted to components or something else). But what about case, when I do want stay with model property name?

According to this issue: emberjs/ember.js#15108 I think it's worth to add such rule.

In short: raise an error when there is a route with uppercased letters in router.js.

@Snik @jbandura @bardzusny @kevinkucharczyk what do you think guys?

The ESLint naming conventions state that rules that prevent some behavior should be prefixed with no-

http://eslint.org/docs/developer-guide/working-with-rules#rule-naming-conventions

Something like no-jquery-ember-run or no-jquery-in-ember-run would be a more appropriate name.

Proposed rule should check if there are any unused dependent properties in computed properties.

Correct:

test: computed('a', 'b', function() {
    return get(this, 'a') + get(this, 'b');
}),

Incorrect:

test: computed('a', 'b', function() {
    return get(this, 'a') + 'asd';
}),

Currently this plugin is not compatible with yeoman generator for eslint: https://github.com/eslint/generator-eslint

New rule is being created in wrong directory instead of the one with other rules.

To be: This plugin's folder structure should be the one recommended in mentioned generator, so that creating new rules with yo eslint:rule is flawless.

This currently results in an alias-model-in-controller error but should probably not:

// app/controllers/things.js
export default Ember.Controller.extend({
  things: Ember.computed.readOnly('model');
});

In almost all cases you should call this._super(...arguments); when overriding methods.

Without inspecting the prototype chain, we can't guard for all possible overwrites, however we can at least guard for known methods provided by Ember. For example, overriding didReceiveAttrs in a component should prompt you to call this._super in that method.

Side note regarding #96: for ES6 classes it's super.didReceiveAttrs(...arguments); or super(...arguments); for the constructor. This does not fully work yet though: https://github.com/rwjblue/ember-decorators/issues/120#issuecomment-314750389

export default Route.extend({
  store: service(),

  model() {
    const store = get(this, 'store');

    return hash({
      pets:   store.findAll('pet'),
      people: store.findAll('people')
    });
  }
});

export default Controller.extend({
  pets:   reads('model.pets'),
  people: reads('model.people')
});

Loading multiple different models via RSVP.hash and aliasing them is a common pattern in Ember applications. Currently alias-model-in-controller does not detect this and prompts the user to alias model.

So I understand the main motivation for named-functions-in-promises is testability. I also prefer named functions for readability in stacktraces/debugging.

However, I currently am of the opinion that using arrow functions to bind context is a bit more readable than just doing a .bind(). Adapting the example from the docs:

export default Component.extend({
  actions: {
    updateUser(user) {
      user.save()
        .then(user => this._reloadUser(user))
        .then(() => this._notifyAboutSuccess())
        .catch(err => this._notifyAboutFailure(err));
        // .then(this._reloadUser.bind(this))
        // .then(this._notifyAboutSuccess.bind(this))
        // .catch(this._notifyAboutFailure.bind(this));
    },
  },

  // ... Imagine there's a lot of other functions here ...

  _reloadUser(user) {
    return user.reload();
  },
  _notifyAboutSuccess() { /* ... */ },
  _notifyAboutFailure(err) { /* ... */ },
});

From my perspective, this still keeps the testability and debuggability (though it will add an extra call to an anonymous function). It has the added benefit of being able to read through the promise chain and know what parameters each function takes without having to jump up and down the file. I'm happy to be persuaded otherwise.

Currently, the rule disallows using the anonymous functions, including arrow function expressions. My proposal would be to also allow arrow function expressions where the body is a CallExpression. To be more strict, can ensure that the method being called belongs to the parent this. Thoughts?

export default Component.extend({
  layout,
  count: 2,
  tabindex: -1,

  someComputedValue: computed.reads('count'),
});

this will generate an error saying that "someComputedValue" single-line function should be above the "tabindex" unknown property

Should ternary statements be classified as default properties? Currently they're considered as an unknown property.

Hi guys,
during running eslint with your plugin I'm getting this error.
The temporary fix for that is disabling the rule but I would like to use it if possible :)

Cannot read property 'map' of null
TypeError: Cannot read property 'map' of null
    at getSegmentNames (/Users/maciej/.nvm/versions/node/v7.9.0/lib/node_modules/eslint-plugin-ember/lib/rules/routes-segments-snake-case.js:32:9)
    at CallExpression.routeOptions.properties.forEach (/Users/maciej/.nvm/versions/node/v7.9.0/lib/node_modules/eslint-plugin-ember/lib/rules/routes-segments-snake-case.js:48:34)
    at Array.forEach (native)
    at Linter.CallExpression (/Users/maciej/.nvm/versions/node/v7.9.0/lib/node_modules/eslint-plugin-ember/lib/rules/routes-segments-snake-case.js:47:35)
    at emitOne (events.js:101:20)
    at Linter.emit (events.js:191:7)
    at NodeEventGenerator.applySelector (/Users/maciej/.nvm/versions/node/v7.9.0/lib/node_modules/eslint/lib/util/node-event-generator.js:265:26)
    at NodeEventGenerator.applySelectors (/Users/maciej/.nvm/versions/node/v7.9.0/lib/node_modules/eslint/lib/util/node-event-generator.js:294:22)
    at NodeEventGenerator.enterNode (/Users/maciej/.nvm/versions/node/v7.9.0/lib/node_modules/eslint/lib/util/node-event-generator.js:308:14)
    at CodePathAnalyzer.enterNode (/Users/maciej/.nvm/versions/node/v7.9.0/lib/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:602:23)
    at Traverser.enter (/Users/maciej/.nvm/versions/node/v7.9.0/lib/node_modules/eslint/lib/linter.js:922:36)

Currently the rules only state "Check order of properties", but we should try to be more helpful and tell the user if the property should be moved up or down, possibly even a line number.

The changes to order-in-models rule seem to want services injected in models to appear at the bottom of the model. This seems like a strange departure since other ordering rules want services to be at the top. Also the example does not show a way to customize where services should appear like in the order-in-controllers rule so I'm not sure if that's even an option or not.

As discussed in #64 we agreed that it might have a sense to add new optional setting, called for example requireThisBinding which would require using current module's methods instead of custom ones.

Example with requireThisBinding = true:

// Good
user.save().then(this._reloadPage.bind(this));

// Good
user.save().then(() => this._reloadPage());

// Bad
user.save().then(reloadPage.bind(this));

// Bad
user.save().then(() => reloadPage());

Default setting would be false so all of the above examples will work fine and we'll keep it backwards compatible.

I don't have any better name for the setting in my mind at this moment, so feel free to add your propositions in comments.

Should computed keys be braced with a preference for longer prefixes?

i.e.

computed('foo.bar.{baz,bash}', 'foo.baz')

is preferred to

computed('foo.{baz,bar.baz,bar.bash}')

this should warn for e.g.:

{
  foo: computed('model.foo', 'model.bar', 'model.baz', 'model.foo', function() {
    // ...
  }
}

This rule should warn about:

import Ember from 'ember';

and e.g.

import Component from 'ember-component';

instead the new module imports should be used, e.g.:

import Component from '@ember/component';

more information is available and linked at https://github.com/ember-cli/ember-rfc176-data

@davidpett encountered an issue (originally discussed in #23) with both of those rules turned on. On of which requires setting queryParams on the very top of a controller, but order-in-controllers rule wants service injections first.That leads to a clear problem.

My first thought is to add queryParams in order-in-controllers rule as the next thing after service injections (to be consistent) and getting rid of query-params-on-top (or at least disabling it in default as well as in recommended config).
That way if anyone would want to change order in his controllers he can do so in eslint's configuration.

Not sure right now if there is a better way of handling this though.

To be discussed.
cc @jbandura @kevinkucharczyk @netes @bardzusny @davidpett

There are 2 options in documentation: 'no-side-effect' in some cases, and 'no-side-effects' in other one.
The correct version is 'no-side-effects'.

Changelogs are especially useful for projects like this where it serves as a go-to place to discover whether new rules have been added or existing rules have been deprecated.

It is generally a good practice to call this._super(...arguments) in Ember's lifecycle hooks, so this rule would enforce its existence. For setup hooks it would enforce it being the first thing called, and in teardown hooks it would enforce it being the last thing called.