Fix undefined / unused variables in spec text. #1403
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1403 +/- ##
=======================================
Coverage 95.70% 95.70%
=======================================
Files 19 19
Lines 11147 11149 +2
Branches 1812 1812
=======================================
+ Hits 10668 10670 +2
Misses 476 476
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Fix a number of small bugs where variables in spec text are used without being defined, defined without being used; a wide range of small bugs exposed by a quick-and-dirty linter I wrote. Common culprits look like: - Referencing undefined _calendar_ instead of _someObject_.[[Calendar]] - Set _thing_ to ? Create... instead of Let _thing_ be ? Create...
cjtenny
force-pushed
the
cjtenny/spec-vardecl-errs
branch
from
cde52aa to
a275020
Compare
Co-authored-by: Jordan Harband <[email protected]>
| 1. Set _fractionString_ to the longest possible substring of _decimalPart_ starting at position 0 and not ending with the code unit 0x0030 (DIGIT ZERO). | ||
| 1. Let _fractionString_ be the longest possible substring of _decimalPart_ starting at position 0 and not ending with the code unit 0x0030 (DIGIT ZERO). |
There was a problem hiding this comment.
Hah, the linter I wrote did not understand that this definition doesn't apply to the use 4 lines down (it's just a scope walk, no control flow). Oops, there's still a bug here, but fixing in #1405 .
There was a problem hiding this comment.
ecmarkup has a linter built in; it might be nice to improve that (or replace it with your linter, if that’s a better direction). cc @bakkot
There was a problem hiding this comment.
This proposal is actually using the linter already!
Re: scope analysis, I've started down that path, but there's a whole bunch of special cases which I've been slowly cleaning up. PRs welcome!
There was a problem hiding this comment.
I handled all of those cases I found in the temporal proposal, which is to say fewer than exist in ecma262, but my current linter is a messy python script thrown together as I encountered things - you probably don't want it in its current state. It was designed as a one-off because I saw recurring issues from what I presume were rebases + copy&paste.
That being said, when I get some extra time I'd love to upstream more of its functionality to ecmarkup; mind giving me some code pointers (IRC?) when I get around to it? I went with the quick python hack because poking around the ecmarkup code left me with some questions.
There was a problem hiding this comment.
Sure! I'm bakkot on IRC in #tc39 and #tc39-delegates; feel free to ping me whenever.
| @@ -759,7 +759,7 @@ <h1>ValidateTemporalDuration ( _years_, _months_, _weeks_, _days_, _hours_, _min | |||
| </emu-clause> | |||
|
|
|||
| <emu-clause id="sec-temporal-defaulttemporallargestunit" aoid="DefaultTemporalLargestUnit"> | |||
| <h1>DefaultTemporalLargestUnit ( _years_, _months_, _weeks_, _days_, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_ )</h1> | |||
| <h1>DefaultTemporalLargestUnit ( _years_, _months_, _weeks_, _days_, _hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_ )</h1> | |||
There was a problem hiding this comment.
I mean yeah but... this is slightly unintuitive... I guess it's OK.
Fix a number of small bugs where variables in spec text are used without being
defined, defined without being used; a wide range of small bugs exposed by a
quick-and-dirty linter I wrote. Common culprits look like: