Page MenuHomePhabricator

"Warning: Too many errors." but none explained
Closed, ResolvedPublic

Description

Hi,

In the following script: https://fr.wiktionary.org/w/index.php?title=Utilisateur:Automatik/test.js&oldid=18725117&action=edit, CodeEditor says there is just 1 warning on the page. However, if I click on the error symbol below the edit window, I've the text: "Warning: Too many errors. (39% scanned)."

So, none error is explained, I just know there are "too many errors". Usually, all errors are explained.

Event Timeline

Automatik raised the priority of this task from to Medium.
Automatik updated the task description. (Show Details)
Automatik changed Security from none to None.
Automatik added a subscriber: Automatik.
Aklapper claimed this task.

Thanks for taking the time to report this!
We are currently migrating from Bugzilla to Phabricator. Currently, bug reports against CodeEditor should still be filed at https://bugzilla.wikimedia.org/enter_bug.cgi?product=MediaWiki%20extensions&component=CodeEditor as linked from https://www.mediawiki.org/wiki/Extension:CodeEditor. Please file your report there.

Aklapper renamed this task from Extension: CodeEditor : too many errors but none explained to "Warning: Too many errors." but none explained.Dec 14 2014, 10:59 PM
Aklapper lowered the priority of this task from Medium to Low.
Aklapper removed a subscriber: Qgil.
Aklapper added a subscriber: Aklapper.
TheDJ added a subscriber: TheDJ.

I suspect this has something to do with the many special characters in that document. For some reason that is tripping something up. It's either an issue in Ace or something that was fixed in a newer version of JSHint.

Reproducible on: http://ace.c9.io/build/kitchen-sink.html
But not on: http://jshint.com (which incidentally uses CodeMiror)

Reproducible on https://ro.wikipedia.org/wiki/MediaWiki:Diacritice.js as well. If this is indeed an issue with the special characters (what does that mean exactly? non-ASCII? curly brackets?), why does it stumble in random locations and not on a "special character"?

Same issue on https://fr.wikipedia.org/wiki/MediaWiki:Common.js despite

jshint  maxerr:600

on the first line. Increasing the value to 900 allow to scan the whole document, but that does not indicate the errors.

But if I add

strict:false

to these jshint parameters, the problem disappear : it does not reveal more errors but only the error explained seams to be counted, the complete file is scanned even without maxerr:600.

Came across this as well - triggered in reftoolbar (https://en.wikipedia.org/wiki/MediaWiki:RefToolbar.js) found this archived topic where it's suggested it's due to Ace being outdated and it's since been fixed* https://www.mediawiki.org/wiki/Topic:Tat8gxmbmjtpn5dl

*fixed in the upstream repository, not in our extension

This appears to still be happening at English Wikipedia (w:en:User:Tol/Sparkle.js).

For me, this can be fixed by adding:

/* jshint maxerr:10000 */

(or with any other number large enough) to the top of the file in question.

https://commons.wikimedia.beta.wmflabs.org/wiki/User:AJ/toomanyerrors.js

"too many errors" is triggered by invisible errors when not using strict mode. The ones that are big enough in numbers to reach the ceiling are typically:

  • Ironic Missing "use strict" statement. errors for every single function.
  • $ is not declared errors. Just repeating $('body'); 101 times also triggers "too many errors".

When coding sloppily there can be others. In JSHint there's this "assume jQuery" option, can that be enabled somehow here as well? (putting /*globals $:false */ in the script also works, but isn't that pretty) Combined with raising the limit to 1000 I suspect this would be sufficient for the vast majority of scripts.

Change 799384 had a related patch set uploaded (by Samtar; author: Samtar):

[mediawiki/extensions/CodeEditor@master] Ace editor config: Increase maxerr

https://gerrit.wikimedia.org/r/799384

Change 799384 abandoned by Samtar:

[mediawiki/extensions/CodeEditor@master] Ace editor config: Increase maxerr

Reason:

876b5b3078a3cacaff091cb636f18ba2b5a6d53e

https://gerrit.wikimedia.org/r/799384

@DannyS712 very kindly figured out how this hook works:

mw.hook( 'codeEditor.configure' ).add(
    function ( s ) {
        s.$worker.call( 'setOptions', [ { maxerr: 1000 } ] );
    }
);

Changing the configuration on the front-end is probably going to be preferred

We could insert this into our standard config setting by adding something like:

editor.session.on('changeMode', function( e, session ) {
  if ( 'ace/mode/javascript' === session.getMode().$id ) {
    if ( session.$worker ) {
      session.$worker.send( 'setOptions', [ {
        'maxerr': 1000
      } ] );
    }
  }
} );

to modules/jquery.codeEditor.js

Change 801429 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/extensions/CodeEditor@master] Configure JS linter to handle more lines

https://gerrit.wikimedia.org/r/801429

Change 801429 merged by jenkins-bot:

[mediawiki/extensions/CodeEditor@master] Configure JS linter to handle more lines

https://gerrit.wikimedia.org/r/801429

TheDJ claimed this task.

We could insert this into our standard config setting by adding something like:

editor.session.on('changeMode', function( e, session ) {
  if ( 'ace/mode/javascript' === session.getMode().$id ) {
    if ( session.$worker ) {
      session.$worker.send( 'setOptions', [ {
        'maxerr': 1000
      } ] );
    }
  }
} );

to modules/jquery.codeEditor.js

Thanks! Just one thing, 1000 won't be enough for the bigger scripts unless jQuery is assumed.

And for reference it might be a good idea to mention this task in a code comment, in case someone in the future wonders "why is this set to 1000?" and changes it.

The patch seems to override not just maxerr but a bunch of things. Crucially, ES6 is now invalid, and globals in browser environments (window, document, etc.), are no longer assumed to be present.

An editor with nothing but

() => {};

now produces

Warning: 'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

And

// jshint undef:true
window;

now produces

Warning: 'window' is not defined.

Both produce

Info: Expected an assignment or function call and instead saw an expression.

Without the patch, both yielded no errors/warnings/info (which I was able to confirm as it's been deployed only on some wikis).

I think we should bump JSHint in Ace (i.e. help github.com/ajaxorg/ace/pull/4610) rather than put a workaround in CodeEditor.

@Nardog as far as declaring ES6 invalid, that's a good thing. You can't have ES6 in gadgets (see also T75714) so the sooner we throw errors towards coders to stop them from developing habits that will prevent their script from ever becoming a gadget, the better.