Validate JavaScript files with ESLint

[angelozerr]

Validate JavaScript files with ESLint by using https://github.com/angelozerr/tern-eslint

[angelozerr]

@vrubezhny @dgolovin @mickaelistria @maxandersen @pascalleclercq @gamerson @paulvi @fbricon @piotrtomiak I have integrated ESLint with a tern plugin. To use it, select eslint inside tern modules.

It's very basic, the ESLint cannot be configurated (it's hard coded) and I think there are some bugs. Please give me your feedback to improve it like add some UI preferences to customize ESLint (how do you see that?) Thanks for your feedback!

[paulvi]

well as there are 3 option for validation: JSDT, JSHint-Eclipse and tern-eslint,
could there be easy switching between for user?

[angelozerr]

JSHint-Eclipse

I'm writing a tern-jshint #242 so tern.java will provide too validation with JSHint.

JSDT Validation belongs to JSDT project so I will not work on this topic. I'm afraid that it's a complex task, because JavaScript Parser ftom JSDT cannot parse ECMAScript5

[maxandersen]

we have disabled more and more of JSDT validation - would be much better if JSDT could be opened up to let other plugin contribute which validation occurs. @vrubezhny we talked about allowing alternative validation and parsers for Mars in JSDT - any progress ?

[angelozerr]

would be much better if JSDT could be opened up to let other plugin contribute which validation occurs

For JSHint and ESLint and Lint (which provides semantic validation), it works with the tern file (ast or text). So in this case, allowing alternative validation and parsers could not help. No?

[maxandersen]

if JSDT could just delegate to Tern then tern would be doing its thing.

I see JSDT as the thing in eclipse that configures the projects etc. but it should not dictate what parser etc. is used (of course its faster/better if only one gets used but since the Java based jscript parser is not ecmascript 5 compatible we need to allow others to do a better job ;)

[angelozerr]

See https://github.com/angelozerr/tern.java/wiki/Tern-Linter-ESLint Now I must create a new plugin which will provide UI preferences to customize ESLInt

[paulvi]

Created #248 Integration with JSDT

[angelozerr]

The integration is done. Next step is to provide an UI preferences to customize tern-eslint.

[paulvi]

Do you mean integration with tern-eslint and not #248 Integration with JSDT ?

[angelozerr]

Do you mean integration with tern-eslint

yes I mean that

[letmaik]

I'm new to tern.java and I can't get eslint to work. It doesn't do any validation (except in html files). When I use jshint for example, everything works. So there is a bug somewhere I think. I use ES6.

[angelozerr]

It doesn't do any validation

I have tried with just:

var

And I have the error [ESLint]: Unexpected end of input.

So there is a bug somewhere I think. I use ES6.

Please give me more info like version of tern.java, your config, your JS code that you try to validate etc. The better thing is that you share your project.

[letmaik]

Eclipse 4.5.0, Tern 1.0.0.201508302102. My project is a general project where I did configure -> add tern. I use a local nodejs installation (0.12.0) on Windows (64bit). In the project settings under Tern, in Modules I enabled ECMAScript (6), Completion Guess, JSDoc, Outline, and ESLint. And in the ESLint settings under Validation, the checkbox "Enable?" is ticked. The project is open source: https://github.com/Reading-eScience-Centre/leaflet-coverage (source code in /leaflet-coverage/ folder).

[angelozerr]

Thanks @neothemachine for your info.

As you use ES6, I suggest really that you install 1.1.0 (see https://github.com/angelozerr/tern.java/wiki/Installation-Update-Site) which provides ES6 support for ES6 class, modules, templates etc.

Please note that the check of ES6 (which is used for completion, navigation, etc) doesn't configure ESLint as ES6. You need to use an ESLint config for that (UI must be improved, to configure this config file, so any feedback are welcome).

After that if you wish to check that it's not buggy, you can see JSON request/response of tern server by opening project properties Tern -> Development and check "Trace on console".

I have tested with 1.1.0, and I have errors from ESLint. Tell me if it works better with 1.1.0 (which is not released)

[letmaik]

I updated to 1.1.0 but it still doesn't work. In the console I see that eslint gets triggered. Output:

-----------------------------------
Tern request#eslint:
{"query":{"type":"eslint","file":"leaflet-coverage/controls/ParameterSync.js"},"files":[{"name":"leaflet-coverage/controls/ParameterSync.js","text":"import L from 'leaflet'\r\nimport wu from 'wu'\r\n\r\n/**\r\n * Default function that checks if two Parameter objects describe\r\n * the same thing. No magic is applied here. Exact match or nothing.\r\n */\r\nfunction defaultMatch (p1, p2) {\r\n  if (!p1.observedProperty.id || !p2.observedProperty.id) {\r\n    return false\r\n  }\r\n  if (p1.observedProperty.id !== p2.observedProperty.id) {\r\n    return false\r\n  }\r\n  if (p1.unit && p2.unit) {\r\n    if (p1.unit.id && p2.unit.id && p1.unit.id !== p2.unit.id) {\r\n      return false\r\n    }\r\n    if (p1.unit.symbol && p2.unit.symbol && p1.unit.symbol !== p2.unit.symbol) {\r\n      return false\r\n    }\r\n  } else if (p1.unit || p2.unit) { // only one of both has units\r\n    return false\r\n  }\r\n  if (p1.categories && p2.categories) {\r\n    if (p1.categories.length !== p2.categories.length) {\r\n      return false\r\n    }\r\n    let idMissing = cat => !cat.id\r\n    if (p1.categories.some(idMissing) || p2.categories.some(idMissing)) {\r\n      return false\r\n    }\r\n    for (let cat1 of p1.categories) {\r\n      if (!p2.categories.some(cat2 => cat1.id === cat2.id)) {\r\n        return false\r\n      }\r\n    }\r\n  } else if (p1.categories || p2.categories) { // only one of both has categories\r\n    return false\r\n  }\r\n  return true\r\n}\r\n\r\n/**\r\n * Synchronizes visualization options of multiple layers with matching Parameter\r\n * and exposes a combined view of those options in form of a virtual layer.\r\n * \r\n * A common use case for this is to have equal palettes and only a single legend\r\n * for multiple layers describing the same parameter.\r\n * \r\n * Synchronizing visualization options means synchronizing certain common properties\r\n * of the layer instances. For example, the palette extents of two layers can be\r\n * synchronized by merging the extents of both. The logic for doing that has to\r\n * be specified in terms of binary functions supplied in the constructor.\r\n * \r\n * By default, a simple algorithm determines if two Parameter objects are equivalent\r\n * by checking whether things like observedPropery have the same ID, units are the same,\r\n * etc. This default algorithm can be replaced with a custom one. Such a custom\r\n * algorithm could relate different vocabularies with each other or perform other checks.\r\n * \r\n * @example <caption>Common palettes</caption>\r\n * let paramSync = new ParameterSync({\r\n *   syncProperties: {\r\n *     palette: (p1, p2) => p1,\r\n *     paletteExtent: (e1, e2) => e1 && e2 ? [Math.min(e1[0], e2[0]), Math.max(e1[1], e2[1])] : null\r\n *   }\r\n * }).on('parameterAdd', e => {\r\n *   // The virtual sync layer proxies the synced palette, paletteExtent, and parameter.\r\n *   // The sync layer will fire a 'remove' event once all real layers for that parameter were removed.\r\n *   let layer = e.syncLayer\r\n *   if (layer.palette) { // \r\n *     new Legend(layer, {\r\n *       position: 'bottomright'\r\n *     }).addTo(map)\r\n *   } \r\n * })\r\n * \r\n */\r\nclass ParameterSync extends L.Class {\r\n  constructor (options) {\r\n    super()\r\n    this._syncProps = options.syncProperties || {}\r\n    this._match = options.match || defaultMatch\r\n    this._paramLayers = new Map() // Map (Parameter -> Set(Layer))\r\n    this._layerListeners = new Map() // Map (Layer -> Map(type -> listener))\r\n    this._propSyncing = new Set() // Set (property name) \r\n  }\r\n  \r\n  addLayer (layer) {\r\n    if (!layer.parameter) {\r\n      return   \r\n    }\r\n    let params = Array.from(this._paramLayers.keys())\r\n    let match = params.find(p => this._match(p, layer.parameter))\r\n    \r\n    let param\r\n    if (!match) {\r\n      param = layer.parameter\r\n      this._paramLayers.set(param, new Set([layer]))\r\n    } else {\r\n      param = match\r\n      this._paramLayers.get(param).add(layer)\r\n      this._syncProperties(param)\r\n    }\r\n    \r\n    this._registerLayerListeners(layer, param)\r\n    \r\n    if (!match) {\r\n      this.fire('parameterAdd', {syncLayer: new SyncLayer(param, this)})\r\n    }\r\n  }\r\n  \r\n  _removeLayer (layer, param) {\r\n    for (let [type, fn] of this._layerListeners.get(layer)) {\r\n      layer.off(type, fn)\r\n    }\r\n    this._layerListeners.delete(layer)\r\n    this._paramLayers.get(param).delete(layer)\r\n    if (this._paramLayers.get(param).size === 0) {\r\n      this._paramLayers.delete(param)\r\n      // underscore since the 'remove' event of the syncLayer should be used\r\n      // from the outside\r\n      this.fire('_parameterRemove', {param: param})\r\n    }\r\n  }\r\n  \r\n  _registerLayerListeners (layer, param) {\r\n    let listeners = new Map([\r\n      ['remove', () => this._removeLayer(layer, param)]\r\n    ])\r\n    for (let prop of Object.keys(this._syncProps)) {\r\n      let type = prop + 'Change' // our convention is camel case\r\n      // TODO does it make sense to unify again, or should it just propagate unchanged?\r\n      listeners.set(type, () => this._syncProperty(param, prop))\r\n    }\r\n    for (let [type, fn] of listeners) {\r\n      layer.on(type, fn)\r\n    }\r\n    this._layerListeners.set(param, listeners)\r\n  }\r\n  \r\n  _syncProperties (param) {\r\n    for (let prop of Object.keys(this._syncProps)) {\r\n      this._syncProperty(param, prop)\r\n    }\r\n  }\r\n  \r\n  _syncProperty (param, prop) {\r\n    if (this._propSyncing.has(prop)) {\r\n      return\r\n    }\r\n    let propreduce = this._syncProps[prop]\r\n    let unified = wu(this._paramLayers.get(param)).map(l => l[prop]).reduce(propreduce)\r\n    // While we unify properties, stop listening for changes to prevent a cycle.\r\n    this._propSyncing.add(prop)\r\n    for (let layer_ of this._paramLayers.get(param)) {\r\n      layer_[prop] = unified\r\n    }\r\n    this._propSyncing.delete(prop)\r\n  }\r\n}\r\n\r\nParameterSync.include(L.Mixin.Events)\r\n\r\nclass SyncLayer extends L.Class {\r\n  constructor (param, paramSync) {\r\n    super()\r\n    this._param = param\r\n    paramSync.on('_parameterRemove', e => {\r\n      if (e.param === param) {\r\n        this.fire('remove')\r\n      }\r\n    })\r\n    let layers = () => paramSync._paramLayers.get(param)\r\n    for (let prop of Object.keys(paramSync._syncProps)) {\r\n      Object.defineProperty(this, prop, {\r\n        get: () => layers().values().next().value[prop],\r\n        set: v => {\r\n          paramSync._propSyncing.add(prop)\r\n          for (let layer of layers()) {\r\n            layer[prop] = v\r\n          }\r\n          paramSync._propSyncing.delete(prop)\r\n          this.fire(prop + 'Change')\r\n        },\r\n        enumerable: true\r\n      })\r\n    }\r\n  }\r\n  \r\n  get parameter () {\r\n    return this._param\r\n  }\r\n}\r\n\r\nSyncLayer.include(L.Mixin.Events)\r\n\r\n// work-around for Babel bug, otherwise ParameterSync cannot be referenced here\r\nexport { ParameterSync as default }\r\n","type":"full"}]}

Tern response#eslint with 34ms:
{"messages":[{"message":"Unexpected reserved word","severity":"error","from":-1,"to":0,"file":"leaflet-coverage/controls/ParameterSync.js"}]}
-----------------------------------

[angelozerr]

With trace it's more easy to understand the problem:)

Ok there is a bug with tern-eslint. When you see:

---

Tern response#eslint with 34ms:

{"messages":[{"message":"Unexpected reserved word","severity":"error","from":-1,"to":0,"file":"leaflet-coverage/controls/ParameterSync.js"}]}

the from is equal to -1, so editor can not display a marker in this position. I will study why tern-eslint returns this value. Please create an issue at https://github.com/angelozerr/tern-eslint/issues

But in your case, you need to configure ESLint with ES6, because today ESlint throws an error when you write import which is not an ES5 keyword. But tern-eslint is not configurable (I had written quiqly by waiting that users like you need it). Please create an another issue with this problem to tern-eslint. Thanks!

[letmaik]

I'm a little confused. You say I cannot configure ESLint with ES6. In the ESLint project property page I can set a path for the eslint.json and for that I set "D:...\eslintrc.json" and eslintrc.json is taken from https://github.com/feross/eslint-config-standard and has "es6": true and others. Isn't that enough?

[angelozerr]

Isn't that enough?

Yes you are right, you must do that, but as I said you, tern-eslint doesn't use this configuration hard coded config https://github.com/angelozerr/tern-eslint/blob/master/eslint.js#L11

I must improve tern-eslint to configure ESlint like I have done for JSHint https://github.com/angelozerr/tern-jshint/blob/master/jshint.js#L59

[angelozerr]

@neothemachine please re-install 1.1.0, your problems should be fixed (I hope).

[letmaik]

Thanks for that, it works. There are some other things but I will create new issues for that. I think this issue can be closed as the basic integration is done.

[gasparmedina]

Do you want to know if eslintignore is supported by this eclipse plugin, and how could you use it?

[angelozerr]

eslintignore is supported by this eclipse plugin,

No, see #438