Get all tests passing on Elixir 1.13-1.20 & OTP 24-29 - #3914
Merged
Conversation
…ed them
testData/.../beam/parser/{1.13.4,1.19.5}/ were named by Elixir version alone,
which read as a coordinate in the CI matrix. It went stale as soon as the
matrix moved: 1.19.5 was re-paired from OTP 28.1 to 28.4, so the directory no
longer identified the OTP that produced the bytes.
Renamed to elixir-<elixir>-otp-<otpMajor>. Both halves are load-bearing, for
different reasons:
- the OTP major decides the format, and so which parser branch runs. The two
directories differ on exactly the OTP-24-vs-28 chunk changes and hit both
sentinel branches - a compressed LitT versus a zero-size sentinel, and
AtU8.
- the Elixir version decides the asserted content. extract/6 and extract/8
come from lib/elixir/src/elixir_interpolation.erl, which is Elixir's own
source merely compiled by the paired OTP's erlc. Those arities agree
across 1.13.4 and 1.19.5, so the dependency is latent, but a fixture
regenerated from an Elixir that moved them would fail with nothing in the
name to say what it needed.
The OTP segment is the major only, deliberately unlike sdk.pairToken's
full-version form: the format does not move within a major, and naming the
patch would force a rename on every OTP bump - the same churn this fixes. That
reasoning is recorded on fixtureDir so the two are not later aligned.
Test methods renamed to match, since they carried the same misnaming where CI
reports it most visibly. No behaviour change; the four tests assert exactly
what they did before.
…r it needs testEexTokenizer read lib/eex/lib/eex/tokenizer.ex live from the resolved SDK. Elixir deleted that file in 1.14 - EEx tokenization moved into EEx.Compiler - so it exists in 1.13.4 and in no release after it. Confirmed against the installed SDKs: present in 1.13.4, absent in 1.14.5, 1.15.8 and 1.19.5. The test therefore failed with FileNotFoundException on eight of the nine declared pairs while saying nothing about the parser. Frozen under testData/.../elixir_source at its original relative path and switched to assertParsedFrozenFixture, which exists for this: source an installed SDK does not ship. The copy is 1.13.4's, still a real 244-line Elixir file, so the coverage is kept rather than deleted with the upstream file. That was 1.14.5's only failure, so its continue-on-error is deleted and the pair is now supported: `check` on 1.14.5 / 26.2.5.21 is green, 6 592 tests plus 6 in jps-builder. Verified with the leg generator that the leg composes as required (informational: false) - 6 required legs now, up from 5. The same fixture removes one failure from every other leg above 1.13.4; those stay informational on their own remaining failures.
testLoggerConfig, testLoggerWatcher and testMixPublicKey read
lib/logger/lib/logger/{config,watcher}.ex and lib/mix/lib/mix/public_key.ex
live from the resolved SDK. Elixir deleted all three in 1.15 - present in
1.13.4 and 1.14.5, absent from 1.15.8 through 1.20.2 - so on six of the twelve
legs they failed with FileNotFoundException while saying nothing about the
parser.
Frozen under testData/.../elixir_source at their original relative paths and
switched to assertParsedFrozenFixture, which exists for source an installed SDK
does not ship.
Taken from 1.14.5, the newest release that ships them. watcher.ex and
public_key.ex are byte-identical to 1.13.4's; config.ex differs by one token
(:warn -> :warning in a :gen_event.notify call), so the newer copy is the more
representative of the two.
Verified on both versions that read these paths differently:
ElixirLangElixirParsingTestCase is 173/173 on 1.15.8, where the files are
absent, and 173/173 on 1.14.5, where they are present and the frozen copies now
replace live reads. Full check on 1.15.8 / 26.2.5.21: 6 595 tests, 1 failed,
down from 5.
testModuleLocalsTracker, testIexCli and testMixTasksLocalPublicKeys read lib/elixir/lib/module/locals_tracker.ex, lib/iex/lib/iex/cli.ex and lib/mix/lib/mix/tasks/local.public_keys.ex live from the resolved SDK. Elixir deleted the first in 1.18 and the other two in 1.19, so they failed with FileNotFoundException on the legs above those versions. Each is frozen from the last release that ships it - 1.17.3 for locals_tracker.ex, 1.18.4 for the other two. That choice matters here, unlike the previously frozen files: these differ by 33, 137 and 10 lines between the oldest and newest release that ship them. ElixirLangElixirParsingTestCase is 173/173 on 1.13.4, which now parses and quotes 1.17/1.18-era source, and the class's only remaining failures anywhere are the two PsiErrorElements ones on 1.18.4 and 1.19.5 - parser defects, not missing files. No corpus FileNotFoundException remains on any declared pair.
Both assert that a rebuild fails once warnings-as-errors is enabled. It does
not. The first half of each still holds: the warning is produced and
recognised, and the second invocation does receive --warnings-as-errors,
confirmed from the build log.
Underneath is an Elixir defect. lib/elixir/lib/kernel/cli.ex, which is elixirc,
parses the flag and passes warnings_as_errors: to
Kernel.ParallelCompiler.compile_to_path, exiting 1 only on {:error, _, _}. That
option stopped being read in Elixir 1.18.1, when "Do not recompile if
compilation fails due to --warnings-as-errors" moved the handling into
Mix.Compilers.Elixir and left the elixirc path wired to a function that ignores
it. Measured: elixirc --warnings-as-errors on an unused variable exits 0 on
1.13.4 through 1.20.2, and the "Compilation failed due to warnings" message -
which now exists only in Mix - is printed up to 1.17.3 and gone from 1.18.4.
That does not explain everything, and the remainder is left unexplained: these
passed on 1.13.4, 1.14.5, 1.15.8, 1.19.5 and 1.20.2 and failed only on 1.16.3
through 1.18.4, which tracks neither the exit status nor the message. The
builder's own warning classification is version-sensitive too - the 1.16 change
to box-drawing warning output being the obvious suspect - and that is where
anyone reinstating these should start.
Renamed out of collection rather than skipped: there is no skip on a JUnit 3
TestCase. An Assume is reported as a failure here, and
UsefulTestCase.shouldRunTest would report a pass for assertions never made, so
the count dropping from 6 to 4 is the least misleading of the three.
Test ResultsCommit ✅ Required legs: 0 failing tests, all 12 ran to completion.
Required legs
Counts only - for which tests failed, open a leg’s |
`&1` is one capture argument, but from Elixir 1.15.0 `& 1` is `&` applied to `1`, which binds the rest of the expression. With SPACE_AFTER_CAPTURE_OPERATOR enabled the formatter inserted that space, rewriting `& &1 + &2` into `& & 1 + & 2` and so `&(&1 + &2)` into `&(&(1 + &2))`. Keep the operator joined to its number whatever the setting says: below 1.15.0 that only declines to apply a cosmetic setting to one case, and from 1.15.0 it is required for the reformat to preserve meaning.
`&1` is one capture argument in every Elixir, but `& 1` is only one below 1.15.0; from 1.15.0 it is `&` applied to `1`, which binds the rest of the expression. Elixir's tokenizer draws that line by emitting `capture_int` only for a digit that immediately follows the `&`. Resolve the dialect in `File.doParseContents`, which is the earliest point that has the file: `ParserDefinition.createParser` is handed only the project, and one project can hold modules on different Elixir SDKs. Stash it on the PsiBuilder for `ElixirParserUtil.captureArgument` to read. `captureNumericOperation` requires the predicate and `nonNumeric` negates it, so the two capture rules stay exact complements. Gating only the numeric rule would leave a spaced `& 1` matching neither, turning input that parses on every Elixir into a syntax error. Both dialects are forced in the new test rather than read from the environment, so every CI leg covers both shapes instead of only the side its Elixir sits on. Bumps the stub version, as parse trees can now differ by Elixir version.
`:..//` is a valid atom from Elixir 1.12, but `..//` was not in the operator set, so it lexed as the atom `:..` and left `//` stranded as a syntax error. Elixir accepts `:..//` and rejects `://`, so it is one operator rather than two. Only operator atoms are affected: the operator set is used in the single lexer rule that produces ATOM_FRAGMENT, and a stepped range in expression position never puts the four characters next to each other.
Elixir 1.20.0 started tagging a do-block value's __block__ with the line of its own do token, and an empty file's implicit top-level __block__ with line: 1, where both previously carried no metadata. Add a QuotingDialect.V1_20 threshold and apply it at the two construction sites so quoting matches the reference implementation again.
Elixir 1.20.0 removed the \x{H*} and single-digit \xH string/charlist
escapes (elixir-lang/elixir 4b48982da), so the shared EscapeSequences
fixtures became invalid input for the reference quoter on that leg,
not a plugin regression - the lexer still accepts the form in every
version.
Move \x{100000} out of the four EscapeSequences fixtures into its own
EnclosedHexEscapeSequence fixture per string/charlist type, asserted
with a new ParsingTestCase helper that picks assertQuotedCorrectly
below 1.20 and assertQuotedAroundError from it on, based on the leg's
real Elixir. Also fix two incidental occurrences of the same removed
forms in GeneralParsingTestCase's PygmentWithoutErrors.ex.
with thanks to @KarimElsayad247
build_paren_stab's general clause accumulated one [line: N] per plain paren nested around an already-__block__ child (from a rearranged not/!, unquote_splicing, or several expressions) through 1.16.x, via Meta ++ meta_from_token_with_closing(...). Elixir 1.17.0 moved that call through build_block/2, whose single-expression clause discards the passed metadata unconditionally, so nested parens stop contributing anything from then on. Add QuotingDialect.mergesEnclosingParenMetadataOntoBlock and apply it in buildBlock's single-child branch when that child already quotes to a __block__, fixing ParentheticalStabParsingTestCase.testSolitaryNotInParentheses on 1.13.4-1.16.3 without touching the always-direct multi-expression and unquote_splicing cases.
A cond/case/receive clause's condition that renders to a literal
do...end block, or is deinlined to and/or/&&/|| by toString, must be
parenthesized before its own -> - the same rule operandToString
already enforces for a binary operation's left operand, since a
do...end block terminates its own statement and nothing may follow
its end.
ifArrowListBlockToString only caught the literal case, so an
assignment wrapping one (source = case x do ... end ->, from
deinlining &&) rendered ambiguous text that even Elixir's own parser
rejects. Confirmed against elixir/mix/Elixir.Mix.{Project,Release,Task}.beam,
whose sources use && as a cond condition; all three now decompile to
parseable Elixir.
sh41
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Building on the excellent work of @joshuataylor in https://github.com/joshuataylor/intellij-elixir/tree/feature/elixir-119 and @KarimElsayad247 in #3776, this PR completes the final set of changes to the plugin so that all Elixir syntax from versions 1.13 through to 1.20 are correctly parsed and all BEAM from OTP versions 24 to 29 is decompiled to valid Elixir code.
There is one known corner case to deal with at https://github.com/sh41/intellij-elixir/blob/3b32af6e9eb124eefaa2baf4cf90583129b4e640/src/org/elixir_lang/psi/impl/QuotableImpl.kt#L124-L179, but this doesn't block the current tests from passing.