Page 2 of 4

Re: JavaScript Linter

Posted: Wed Dec 13, 2023 11:40 pm
by phkb
Couple of things:
1. All those "ai" issues seem to be because the Linter is not able to determine that "ai" has actually been defined.

Code: Select all

	this.ai = new worldScripts["oolite-libPriorityAI"].PriorityAIController(this.ship);
	ai.setParameter("oolite_flag_watchForCargo",true);
In the first line, "ai" is defined in the context of "this". The second line, I think "this" is assumed from the runtime compiler's point of view. But the Linter can't seem to make that assumption. In any case, it's not an error or even a warning.

2. Timer is a valid object so this:

Code: Select all

2023-12-13T20:41:43.0865984Z   34:24  error  'Timer' is not defined  no-undef
is not a valid error. Can we add it to the whitelist of objects?

3. These are really weird:

Code: Select all

2023-12-13T20:41:43.0867678Z   30:10  error  'worldScripts' is already defined as a built-in global variable  no-redeclare
2023-12-13T20:41:43.0868493Z   30:10  error  'worldScripts' is defined but never used                         no-unused-vars

2023-12-13T20:41:43.0870253Z   30:10  error  'galaxyNumber' is already defined as a built-in global variable      no-redeclare
2023-12-13T20:41:43.0871078Z   30:24  error  'missionVariables' is already defined as a built-in global variable  no-redeclare
2023-12-13T20:41:43.0871893Z   30:42  error  'system' is already defined as a built-in global variable            no-redeclare
The lines being referred to are comments, not executable code. Why is the Linter interpreting them as code?

There are more errors where the Linter has not understood valid Oolite objects (log, System, displayNameForCommodity, SoundSource, etc). Unless we whitelist all of these the error list is going to long.

Re: JavaScript Linter

Posted: Thu Dec 14, 2023 7:05 am
by hiran
re 1)
I am not conivnced the linter is wrong although it looks like. There is a difference between this.ai and ai, and there may be situations where it is meaningful. I also agree these are bad situation where an instance variable is shadowed by a local variable.

Yet I am not sure how to best deal with the message. Would we have to change all references to this.ai?

re 2)
I added all the Oolite classes from the wiki's scripting model meanwhile. The result is on Github - it affected few lines in the log only so I did not post another one.
Consider it 'done'

re 3)
The linter has several levels where you can configure it. I am using a configfile that is global for the project, but you can also configure it per source file. Someone who used jslint has done so a long time ago. That's why you find specific comments that are ignored by JavaScript but interpreted by the linter. Apparently eslint also reacts to the jslint comments.

So we have a situation where the source file defines some global variables that I already added to the global configuration. And one of the linter's rules disallows redefinition.

One solution could be to remove the global configuration - which gives us errors in some situations.
One solution could be to remove the redefinition comments in the source that gives us a consistent setup easily to be transferred to OXP proijects.
One solution could be to tell the linter that redefinition is allowed.

Which of them should we take?

Re: JavaScript Linter

Posted: Thu Dec 14, 2023 7:41 am
by phkb
hiran wrote: Thu Dec 14, 2023 7:05 am
Yet I am not sure how to best deal with the message. Would we have to change all references to this.ai?
I think if we wanted to get a clean report from the Linter, yes, that's exactly what we'd have to do. But because it's working as-is, I don't want to touch it.
hiran wrote: Thu Dec 14, 2023 7:05 am
Which of them should we take?
Of the three options, the second would be my preferred one.

Re: JavaScript Linter

Posted: Thu Dec 14, 2023 8:35 am
by hiran
phkb wrote: Thu Dec 14, 2023 7:41 am
hiran wrote: Thu Dec 14, 2023 7:05 am
Yet I am not sure how to best deal with the message. Would we have to change all references to this.ai?
I think if we wanted to get a clean report from the Linter, yes, that's exactly what we'd have to do. But because it's working as-is, I don't want to touch it.
I'd like to have a clean report and at the same time am reluctant to change the working code. First I shall get advice from the eslint community.
phkb wrote: Thu Dec 14, 2023 7:41 am
hiran wrote: Thu Dec 14, 2023 7:05 am
Which of them should we take?
Of the three options, the second would be my preferred one.
Same here. Although that implies we touch the working code and remove the unneeded comments.

Let's see what I get from eslint.

Re: JavaScript Linter

Posted: Thu Dec 14, 2023 10:58 am
by cim
hiran wrote: Thu Dec 14, 2023 7:05 am
re 1)
I am not conivnced the linter is wrong although it looks like. There is a difference between this.ai and ai, and there may be situations where it is meaningful. I also agree these are bad situation where an instance variable is shadowed by a local variable.

Yet I am not sure how to best deal with the message. Would we have to change all references to this.ai?
Simplest would probably be just to add an

Code: Select all

var ai = this.ai;
line immediately after the definition of this.ai in each affected AI script.

That should be safe, short, and fix all those errors.
(It having been long enough that I've forgotten a lot of the details of OOJS, I'm surprised *now* that it ever worked without that)

Re: JavaScript Linter

Posted: Thu Dec 14, 2023 11:03 pm
by hiran
cim wrote: Thu Dec 14, 2023 10:58 am
Simplest would probably be just to add an

Code: Select all

var ai = this.ai;
line immediately after the definition of this.ai in each affected AI script.

That should be safe, short, and fix all those errors.
(It having been long enough that I've forgotten a lot of the details of OOJS, I'm surprised *now* that it ever worked without that)
Definitely a way to go. :-)

Your surprise would actually tell me that there is something the linter spotted.
BTW, do you happen to know when/who applied a linter on that source already? And maybe a reason why that was not kept up?

Re: JavaScript Linter

Posted: Fri Dec 15, 2023 8:52 am
by cim
Oh, the linter is definitely right about that one being at the very least good practice to fix.

No idea when or who last ran one on it, though. It wasn't something I noticed when I started looking at the core code.

Re: JavaScript Linter

Posted: Fri Dec 15, 2023 5:04 pm
by hiran
So I applied the suggested fix for the undefined ai and removed the in-source linter configuration.

We have a lot less error messages now, so here is the complete log:

Code: Select all

oolite/DebugOXP/Debug.oxp/Scripts/oolite-debug-console.js
  258:6  error  Parsing error: Unexpected token [

oolite/Resources/Scripts/oolite-contracts-cargo.js
  189:37  error  'to' is defined but never used            no-unused-vars
  189:41  error  'from' is defined but never used          no-unused-vars
  195:38  error  'from' is defined but never used          no-unused-vars
  281:7   error  Empty block statement                     no-empty
  285:7   error  Empty block statement                     no-empty
  419:40  error  'interfaceKey' is defined but never used  no-unused-vars
  454:13  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  455:13  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  456:13  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  457:13  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  596:12  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  597:12  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  598:12  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  599:12  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  600:12  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  601:12  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  885:70  error  'displayNameForCommodity' is not defined  no-undef
  908:9   error  'spareSafe' is already defined            no-redeclare

oolite/Resources/Scripts/oolite-contracts-parcels.js
  197:37  error  'to' is defined but never used            no-unused-vars
  197:41  error  'from' is defined but never used          no-unused-vars
  203:38  error  'from' is defined but never used          no-unused-vars
  409:41  error  'interfaceKey' is defined but never used  no-unused-vars
  444:13  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  445:13  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  446:13  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  447:13  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  574:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  575:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  576:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  577:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  578:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  579:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs

oolite/Resources/Scripts/oolite-contracts-passengers.js
  186:37  error  'to' is defined but never used            no-unused-vars
  186:41  error  'from' is defined but never used          no-unused-vars
  192:38  error  'from' is defined but never used          no-unused-vars
  319:14  error  'j' is already defined                    no-redeclare
  408:44  error  'interfaceKey' is defined but never used  no-unused-vars
  443:14  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  444:14  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  445:14  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  446:14  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  447:14  error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  583:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  584:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  585:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  586:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  587:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  588:6   error  Mixed spaces and tabs                     no-mixed-spaces-and-tabs

oolite/Resources/Scripts/oolite-equipment-control.js
   65:51  error  'item' is defined but never used  no-unused-vars
   70:52  error  'item' is defined but never used  no-unused-vars
  240:57  error  'info' is defined but never used  no-unused-vars
  247:58  error  'info' is defined but never used  no-unused-vars

oolite/Resources/Scripts/oolite-global-prefix.js
  165:1  error  Parsing error: The keyword 'const' is reserved

oolite/Resources/Scripts/oolite-locale-functions.js
  43:1  error  Parsing error: The keyword 'const' is reserved

oolite/Resources/Scripts/oolite-nova-mission.js
  361:11  error  Expected a conditional expression and instead saw an assignment  no-cond-assign

oolite/Resources/Scripts/oolite-populator.js
  2357:4  error  Parsing error: The keyword 'let' is reserved

oolite/Resources/Scripts/oolite-primable-equipment-manager.js
  175:11  error  'i' is already defined  no-redeclare

oolite/Resources/Scripts/oolite-priorityai.js
   121:4   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   179:10  error  'branch' is already defined                no-redeclare
   194:3   error  Unnecessary semicolon                      no-extra-semi
   226:3   error  Unnecessary semicolon                      no-extra-semi
   240:3   error  Unnecessary semicolon                      no-extra-semi
   279:12  error  'i' is already defined                     no-redeclare
   388:9   error  'message' is already defined               no-redeclare
   756:7   error  'gs' is already defined                    no-redeclare
   761:12  error  'i' is already defined                     no-redeclare
   763:8   error  'spd' is already defined                   no-redeclare
   815:9   error  'crew' is assigned a value but never used  no-unused-vars
  1123:7   error  'gs' is already defined                    no-redeclare
  1143:7   error  'gs' is already defined                    no-redeclare
  1155:7   error  'gs' is already defined                    no-redeclare
  1548:12  error  'i' is already defined                     no-redeclare
  1558:7   error  'gs' is already defined                    no-redeclare
  1559:12  error  'i' is already defined                     no-redeclare
  2472:6   error  'gs' is already defined                    no-redeclare
  2473:11  error  'i' is already defined                     no-redeclare
  3110:9   error  'launchpos' is already defined             no-redeclare
  3267:13  error  'i' is already defined                     no-redeclare
  3297:7   error  'rt' is already defined                    no-redeclare
  4006:12  error  'i' is already defined                     no-redeclare
  4020:7   error  'gs' is already defined                    no-redeclare
  4021:12  error  'i' is already defined                     no-redeclare
  4122:12  error  'i' is already defined                     no-redeclare
  4136:7   error  'gs' is already defined                    no-redeclare
  4137:12  error  'i' is already defined                     no-redeclare
  4998:88  error  'cargo' is defined but never used          no-unused-vars
  4998:94  error  'ship' is defined but never used           no-unused-vars
  5606:69  error  'displayNameForCommodity' is not defined   no-undef
  5613:85  error  'target' is defined but never used         no-unused-vars
  5663:90  error  'message' is defined but never used        no-unused-vars
  5775:90  error  'message' is defined but never used        no-unused-vars
  5781:91  error  'weapon' is defined but never used         no-unused-vars
  5949:84  error  'target' is defined but never used         no-unused-vars

oolite/Resources/Scripts/oolite-thargoid-plans-mission.js
  106:5  error  Mixed spaces and tabs  no-mixed-spaces-and-tabs

oolite/Resources/Scripts/oolite-tutorial.js
   210:43  error  'whom' is defined but never used           no-unused-vars
   210:49  error  'type' is defined but never used           no-unused-vars
   246:42  error  'station' is defined but never used        no-unused-vars
   275:50  error  'delta' is defined but never used          no-unused-vars
   277:7   error  '$blockTorus' is not defined               no-undef
   434:2   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   435:2   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   436:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   437:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   438:4   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   439:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   440:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   441:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   442:4   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   443:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   444:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   445:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   446:4   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   447:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   448:3   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   449:2   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   493:38  error  'delta' is defined but never used          no-unused-vars
   494:8   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   495:9   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   496:9   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   497:8   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   828:7   error  'buoy' is assigned a value but never used  no-unused-vars
   860:55  error  'type' is defined but never used           no-unused-vars
   939:1   error  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
  1284:7   error  'buoy' is assigned a value but never used  no-unused-vars
  1499:35  error  'delta' is defined but never used          no-unused-vars
  1613:33  error  'delta' is defined but never used          no-unused-vars

Re: JavaScript Linter

Posted: Sat Dec 16, 2023 9:29 pm
by hiran
Most of the unused variables were actually function parameters that had to be there since we have event handlers. If they do not all get used, what can you do? Well, eslint allows to ignore arguments that follow a specific pattern. I configured it and changed the unused arguments to follow that pattern: They start with an underscore now.

I also looked up the 'no-mixed-spaces-and-tabs' rule. It is marked as deprecated in the eslint documentation, yet it is active per default. I reconfigured eslint to mention it but not break the build - which means you can see it as warning now.

With that, we are down to these messages:

Code: Select all

oolite/DebugOXP/Debug.oxp/Scripts/oolite-debug-console.js
  258:6  error  Parsing error: Unexpected token [

oolite/Resources/Scripts/oolite-contracts-cargo.js
  281:7   error    Empty block statement                     no-empty
  285:7   error    Empty block statement                     no-empty
  454:13  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  455:13  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  456:13  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  457:13  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  596:12  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  597:12  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  598:12  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  599:12  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  600:12  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  601:12  warning  Mixed spaces and tabs                     no-mixed-spaces-and-tabs
  885:70  error    'displayNameForCommodity' is not defined  no-undef
  908:9   error    'spareSafe' is already defined            no-redeclare

oolite/Resources/Scripts/oolite-contracts-parcels.js
  444:13  warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  445:13  warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  446:13  warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  447:13  warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  574:6   warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  575:6   warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  576:6   warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  577:6   warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  578:6   warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs
  579:6   warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs

oolite/Resources/Scripts/oolite-contracts-passengers.js
  319:14  error    'j' is already defined  no-redeclare
  443:14  warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  444:14  warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  445:14  warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  446:14  warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  447:14  warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  583:6   warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  584:6   warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  585:6   warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  586:6   warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  587:6   warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs
  588:6   warning  Mixed spaces and tabs   no-mixed-spaces-and-tabs

oolite/Resources/Scripts/oolite-global-prefix.js
  165:1  error  Parsing error: The keyword 'const' is reserved

oolite/Resources/Scripts/oolite-locale-functions.js
  43:1  error  Parsing error: The keyword 'const' is reserved

oolite/Resources/Scripts/oolite-nova-mission.js
  361:11  error  Expected a conditional expression and instead saw an assignment  no-cond-assign

oolite/Resources/Scripts/oolite-populator.js
  2357:4  error  Parsing error: The keyword 'let' is reserved

oolite/Resources/Scripts/oolite-primable-equipment-manager.js
  175:11  error  'i' is already defined  no-redeclare

oolite/Resources/Scripts/oolite-priorityai.js
   121:4   warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   179:10  error    'branch' is already defined                no-redeclare
   194:3   error    Unnecessary semicolon                      no-extra-semi
   226:3   error    Unnecessary semicolon                      no-extra-semi
   240:3   error    Unnecessary semicolon                      no-extra-semi
   279:12  error    'i' is already defined                     no-redeclare
   388:9   error    'message' is already defined               no-redeclare
   756:7   error    'gs' is already defined                    no-redeclare
   761:12  error    'i' is already defined                     no-redeclare
   763:8   error    'spd' is already defined                   no-redeclare
   815:9   error    'crew' is assigned a value but never used  no-unused-vars
  1123:7   error    'gs' is already defined                    no-redeclare
  1143:7   error    'gs' is already defined                    no-redeclare
  1155:7   error    'gs' is already defined                    no-redeclare
  1548:12  error    'i' is already defined                     no-redeclare
  1558:7   error    'gs' is already defined                    no-redeclare
  1559:12  error    'i' is already defined                     no-redeclare
  2472:6   error    'gs' is already defined                    no-redeclare
  2473:11  error    'i' is already defined                     no-redeclare
  3110:9   error    'launchpos' is already defined             no-redeclare
  3267:13  error    'i' is already defined                     no-redeclare
  3297:7   error    'rt' is already defined                    no-redeclare
  4006:12  error    'i' is already defined                     no-redeclare
  4020:7   error    'gs' is already defined                    no-redeclare
  4021:12  error    'i' is already defined                     no-redeclare
  4122:12  error    'i' is already defined                     no-redeclare
  4136:7   error    'gs' is already defined                    no-redeclare
  4137:12  error    'i' is already defined                     no-redeclare
  5606:69  error    'displayNameForCommodity' is not defined   no-undef

oolite/Resources/Scripts/oolite-thargoid-plans-mission.js
  106:5  warning  Mixed spaces and tabs  no-mixed-spaces-and-tabs

oolite/Resources/Scripts/oolite-tutorial.js
   277:7  error    '$blockTorus' is not defined               no-undef
   434:2  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   435:2  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   436:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   437:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   438:4  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   439:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   440:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   441:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   442:4  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   443:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   444:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   445:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   446:4  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   447:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   448:3  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   449:2  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   494:8  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   495:9  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   496:9  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   497:8  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
   828:7  error    'buoy' is assigned a value but never used  no-unused-vars
   939:1  warning  Mixed spaces and tabs                      no-mixed-spaces-and-tabs
  1284:7  error    'buoy' is assigned a value but never used  no-unused-vars

Re: JavaScript Linter

Posted: Mon Dec 18, 2023 9:48 pm
by hiran
No further comments? Then this setup could be ready to check OXPs? I'm just wondering how to best establish that.

We could run the linter when indexing all the OXPs. But that would be after an expansion has been published on the Expansion Manager.
Es there a way to be earlier in the OXP development process?

Re: JavaScript Linter

Posted: Tue Dec 19, 2023 1:11 am
by phkb
"displayNameForCommodity" is a reserved word (or should be). I don't know why "mixed spaces and tabs" is an issue. I'd turn that one off, honestly.

As for when this should be run, it has to be an opt-in process, and not during the indexing phase, which already takes enough time to run. If there was a way that a user could selectively run the process on an OXZ that would be ideal. I can just see there would be a lot of unnecessary noise (ie. pages and pages of error messages) if we forced it to run over everything all the time.

If we could have an online tool, like https://validatejavascript.com/, which understood the Oolite programming model, we could point people in that direction so they could easily check their code files without adding a processing burden to the indexer.

Re: JavaScript Linter

Posted: Tue Dec 19, 2023 5:33 am
by hiran
phkb wrote: Tue Dec 19, 2023 1:11 am
"displayNameForCommodity" is a reserved word (or should be). I don't know why "mixed spaces and tabs" is an issue. I'd turn that one off, honestly.
I'll change that - no problem.
phkb wrote: Tue Dec 19, 2023 1:11 am
As for when this should be run, it has to be an opt-in process, and not during the indexing phase, which already takes enough time to run. If there was a way that a user could selectively run the process on an OXZ that would be ideal. I can just see there would be a lot of unnecessary noise (ie. pages and pages of error messages) if we forced it to run over everything all the time.

If we could have an online tool, like https://validatejavascript.com/, which understood the Oolite programming model, we could point people in that direction so they could easily check their code files without adding a processing burden to the indexer.
Maybe @timer can help. We can install the linter with the configuration created here such that is is invoked via web request. Then users could upload single JS files or possibly the whole OXZ to get it validated.

Re: JavaScript Linter

Posted: Tue Dec 19, 2023 8:02 am
by hiran
Here's the current messages:

Code: Select all

oolite/DebugOXP/Debug.oxp/Scripts/oolite-debug-console.js
  258:6  error  Parsing error: Unexpected token [

oolite/Resources/Scripts/oolite-contracts-cargo.js
  281:7  error  Empty block statement           no-empty
  285:7  error  Empty block statement           no-empty
  908:9  error  'spareSafe' is already defined  no-redeclare

oolite/Resources/Scripts/oolite-contracts-passengers.js
  319:14  error  'j' is already defined  no-redeclare

oolite/Resources/Scripts/oolite-global-prefix.js
  165:1  error  Parsing error: The keyword 'const' is reserved

oolite/Resources/Scripts/oolite-locale-functions.js
  43:1  error  Parsing error: The keyword 'const' is reserved

oolite/Resources/Scripts/oolite-nova-mission.js
  361:11  error  Expected a conditional expression and instead saw an assignment  no-cond-assign

oolite/Resources/Scripts/oolite-populator.js
  2357:4  error  Parsing error: The keyword 'let' is reserved

oolite/Resources/Scripts/oolite-primable-equipment-manager.js
  175:11  error  'i' is already defined  no-redeclare

oolite/Resources/Scripts/oolite-priorityai.js
   179:10  error  'branch' is already defined                no-redeclare
   194:3   error  Unnecessary semicolon                      no-extra-semi
   226:3   error  Unnecessary semicolon                      no-extra-semi
   240:3   error  Unnecessary semicolon                      no-extra-semi
   279:12  error  'i' is already defined                     no-redeclare
   388:9   error  'message' is already defined               no-redeclare
   756:7   error  'gs' is already defined                    no-redeclare
   761:12  error  'i' is already defined                     no-redeclare
   763:8   error  'spd' is already defined                   no-redeclare
   815:9   error  'crew' is assigned a value but never used  no-unused-vars
  1123:7   error  'gs' is already defined                    no-redeclare
  1143:7   error  'gs' is already defined                    no-redeclare
  1155:7   error  'gs' is already defined                    no-redeclare
  1548:12  error  'i' is already defined                     no-redeclare
  1558:7   error  'gs' is already defined                    no-redeclare
  1559:12  error  'i' is already defined                     no-redeclare
  2472:6   error  'gs' is already defined                    no-redeclare
  2473:11  error  'i' is already defined                     no-redeclare
  3110:9   error  'launchpos' is already defined             no-redeclare
  3267:13  error  'i' is already defined                     no-redeclare
  3297:7   error  'rt' is already defined                    no-redeclare
  4006:12  error  'i' is already defined                     no-redeclare
  4020:7   error  'gs' is already defined                    no-redeclare
  4021:12  error  'i' is already defined                     no-redeclare
  4122:12  error  'i' is already defined                     no-redeclare
  4136:7   error  'gs' is already defined                    no-redeclare
  4137:12  error  'i' is already defined                     no-redeclare

oolite/Resources/Scripts/oolite-tutorial.js
   277:7  error  '$blockTorus' is not defined               no-undef
   828:7  error  'buoy' is assigned a value but never used  no-unused-vars
  1284:7  error  'buoy' is assigned a value but never used  no-unused-vars
Does anyone know what the parsing error is? It can't since the code has been running for too long, yet it should be a serious problem.

Re: JavaScript Linter

Posted: Tue Dec 19, 2023 8:25 am
by phkb
This is the line

Code: Select all

	var [key, value] = string.getOneToken();
It’s a form I’ve never seen before, taking the array return object (from getOneToken) and assigning the sub elements inside the brackets. I mean, obviously it works, but I would have written like this
var ary = string.getOneToken();
var key = ary[0], value = ary[1];

Re: JavaScript Linter

Posted: Tue Dec 19, 2023 7:20 pm
by hiran
phkb wrote: Tue Dec 19, 2023 8:25 am
This is the line

Code: Select all

	var [key, value] = string.getOneToken();
It’s a form I’ve never seen before, taking the array return object (from getOneToken) and assigning the sub elements inside the brackets. I mean, obviously it works, but I would have written like this
var ary = string.getOneToken();
var key = ary[0], value = ary[1];
But... is this valid JavaScript? Or is this one of the customizations that were done for Oolite?
I wonder why eslint did not recognize that line.