Don't lint non-angular files

[ptarjan]

I tried using your plugin, but part of my repo is angular and another part is just normal JS files. Can you only apply your rules if there is a angular.module() in the chain or something smart?

[tilmanschweitzer]

What kind of false positives do you have?

[ptarjan]

Tons. Here are a few samples:

   4:1   error  You should use the $window service instead of the default window object      angular/window-service

  114:7   error  You should use the "error" method of the AngularJS Service $log instead of the console object  angular/log

  19:11  error  You should use the toJson method instead of JSON.stringify                                   angular/json-functions

   674:26  error    You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined  angular/definedundefined

  244:33  warning  You should use angular.element instead of the jQuery $ object                                angular/angularelement

  25:15  error    You should use the angular.isArray method                      angular/typecheck-array

[remcohaszing]

It's not possible to determine which files you want to lint. This is a personal preference.

If your code base separates the angular part and the non-angular part, you could implement this yourself though.

Assuming you have the following layout

app/
 |- angular-app/
 |   `- .eslintrc
 |- non-angular/
 |   `- .eslintrc
 `- .eslintrc

Where app/.eslintrc contains your common settings, app/non-angular/.eslintrc contains your non-angular specific rules and app/angular-app/.eslintrc contains:

plugins:
  - angular

I use a similar approach to distinguish between the rules for my configuration files (gulpfile, Karma configuration, etc.) and my AngularJS source files.

[ptarjan]

I often intermix pure JS modules in the same dir as my angular services or controllers.

Could this plugin just make sure there is a user of angular.module in the file before it reports anything?

[EmmanuelDemey]

I think it is not possible to detect code inside angular.module, because developers can use AMD or CJS module.
We can define the implementation of the AngularJS component in a file, and include it in the AngularJS architecture (via filter/directive/service/controller..) in another file.
In the first file, we do not have any angular.module call, so it is not possible to detect AngularJS code in this way.

[ptarjan]

Can you only lint inside a angular.controller and angular.service etc.?

Sent from my iPhone

On Oct 8, 2015, at 2:29 AM, Emmanuel DEMEY [email protected] wrote:

I think it is not possible to detect code inside angular.module, because developers can use AMD or CJS module.
We can define the implementation of the AngularJS component in a file, and include it in the AngularJS architecture (via filter/directive/service/controller..) in another file.
In the first file, we do not have any angular.module call, so it is not possible to detect AngularJS code in this way.

—
Reply to this email directly or view it on GitHub.

[EmmanuelDemey]

As I tried to explain I think it is not possible. For exemple, imagine I have an ES2015 service class in a service.js file :

export class Service {}

And in other file, I register this service into Angular via an application.js file :

import {Service} from 'myModule'
angular.module('Service', Service)

How can we detect, in the service.js, that the class is in fact an AngularJS Service component ? I think it is impossible.

Manu

[ptarjan]

I agree. JavaScript is Turing complete so you can't know if something will be registered as a controller unless you execute it. But as with all static analysis, you do your best. My vote is to err on the side of false negatives not false positives for a linter. So only yell if you are sure it is a controller or service or whatever and the slowly get smarter about detecting them.

Sent from my iPhone

On Oct 11, 2015, at 6:35 AM, Emmanuel DEMEY [email protected] wrote:

As I tried to explain I think it is not possible. For exemple, imagine I have an ES2015 service class in a service.js file :

export class Service {}
And in other file, I register this service into Angular via an application.js file :

import {Service} from 'myModule'
angular.module('Service', Service)
How can we detect, in the service.js, that the class is in fact an AngularJS Service component ? I think it is impossible.

Manu

—
Reply to this email directly or view it on GitHub.

[tilmanschweitzer]

We could add a global setting and turn off the rules based on some heuristics. The detection might be incomplete, but it's a tradeoff.

"settings": {
  "angular/detect-angular-context": ["module-setter"]
}

It think the simplest way it to look for a module getter or setter and then use the rules for the complete file. It might be also possible to check all parent nodes for a certain node, but I pretty sure this will cause performance issues. A file based approach is simpler.

[remcohaszing]

I think it is a per rule setting which ones should be enabled.

I agree with @ptrarjan that the following errors should only be detected when inside of an AngularJS component:

   4:1   error  You should use the $window service instead of the default window object      angular/window-service

  114:7   error  You should use the "error" method of the AngularJS Service $log instead of the console object  angular/log

However, these rules could still apply when used outside of an AngularJS component. This is purely a preference. These rules can be disabled using /* eslint-disable angular/<rule_name> */.

  19:11  error  You should use the toJson method instead of JSON.stringify                                   angular/json-functions

   674:26  error    You should not use directly the "undefined" keyword. Prefer angular.isUndefined or angular.isDefined  angular/definedundefined

  244:33  warning  You should use angular.element instead of the jQuery $ object                                angular/angularelement

  25:15  error    You should use the angular.isArray method                      angular/typecheck-array

I personally don't have experience yet using ES6 specific syntax such as classes combined with AngularJS. ngAnnotate has support for ES6 and TypeScript. I think the 'ngInject'; annotation can be used to detect services or controllers from classes.

[mradionov]

Another solution could be a disabling of the entire plugin from within a file via inline comments, but it seems to be not supported by eslint yet (eslint/eslint#3419, eslint/eslint#2118)

[tilmanschweitzer]

... or the angular components could be ignored using some filename conventions.

"settings": {
    "angular/file-name-pattern": "/\w+\.(module|service|controller|directive|filter)\.js/"
}

[tilmanschweitzer]

Another suggestion. We provide different configurable heuristics for detecting angular files.

"settings": {
    "angular/only-lint-matching-files": {
        // configured heuristic
   }
}

Ideas for such options...

• "code-patterns": ["module-setters", "module-getters", "angular-object"]
  • ["module-setters"] - presence of a module setter
  • ["module-getters"] - presence of a module getter
  • ["angular-object"] - any use of the angular object
• "file-name-patter": ["\.(module|service|controller|directive|filter)\.js$"]

If we find better ways to detect angular files we can easily add an options to this list.

The "code-pattern" option will cover the usecase of @ptarjan but it is also possible to write angular components without any reference to the framework with the "file-name-pattern" option.

[mradionov]

I've added a proof of concept to my fork (here), which kinda allows to control whether or not plugin rules will be executed. It looks hacky, but might work. Basically it uses local variable to block rule execution from within a plugin, it resets the variable after each file ends and updates variable value when rule is enabled again. The following rule should block all angular rules for for current file:

/* eslint "angular/disable-plugin": 2 */

It does not work in alignment with ESLint API and may lead to bugs in plugin and maybe even ESLint, I agree if it won't end up in the plugin, but is one of the alternatives.

[tilmanschweitzer]

@mradionov Nice. Your proof of concept seems to work, but as you already said, it is a little hacky and I fear it may be hard to predict side effects.

I took a part of your approach to disabled rules by a filename pattern as another proof of concept.

[mradionov]

If anybody is interested, I wrote a plugin for ESLint, which allows to disable plugins per file via inline comments https://github.com/mradionov/eslint-plugin-disable

[remcohaszing]

@mradionov Nice! I think this is the best solution by far.

I'm closing this issue, because I think this fixes it. I don't think it should be part of an AngularJS plugin to handle this.

If anyone disagrees, just reopen it.

[tilmanschweitzer]

@remcohaszing I also like the solution and we should mention it in the docs, but I also think that some convention based approach is necessary, if a project has a lot on non angular resources.