Take a comment-then-code line's layout from the code's column

[pcshrosbree]

Stacked on #250 (comment-only lines). This PR's own change is the second commit, "Take a comment-then-code line's layout from the code's column".

A line that starts left of the current scope with block comments and then holds code decides layout from the comment's column:

type T() =
        member this.A() = 1
(* c *) member this.C() = 3
let g = 2

On main, the (* at column 0 closes the class, and member this.C is parsed outside T, with no ERROR node. F#'s offside rule ignores comments: FSC's lexer drops them before the layout filter, so the line's column is that of member, 8, and member this.C is the class's second member. The same happens in a function body, between match arms, and on the closing line of a multi-line comment (b *) member this.C() = 3).

#250 handles a line that holds only comments: the scan declines and the next line decides. It leaves this case for a separate change, because the column has to come from the code after the comments.

Change

common/scanner.h, at the DEDENT decision, where #250 handles the comment-only line. When the line starts left of the scope with block comments followed by code, comments_fill_rest_of_line() now also reports three things about what follows the comments: the code's column, its first character, and whether the comments are ambiguous (below).

The code's first token is classified as the ordinary line-start path classifies it. The helper reports the first character because it has already consumed a leading ( (to see whether another comment follows) or / (to see whether it is //). A word is read once and whole, and is one of:

• a layout keyword;
• the operator or;
• a member keyword (member, override, static, abstract, default, val);
• an ordinary word, which is never an infix operator, whatever follows it.

A word runs over every identifier character, non-ASCII letters included, so oré is an ordinary word, not or followed by something. After |, a pattern makes the line a match arm, as on the ordinary path; a non-ASCII letter also starts one (|é -> 2).

Then:

• An infix operator continues the current item, wherever it is: F# lets an operator undent.
• Other code takes the code's column when an open scope sits exactly at that column. At the current scope's column it starts a new item; left of it, it closes the scopes above that level.
• A member keyword takes the code's column, under the same condition, only after comments that start left of every open scope but the outermost. F# ends any member body before a member keyword, even one whose scope sits at the keyword's column: FCS gives two members for member x.A =, 1 on the next line, then (* c *) member x.B = 2 with member at the 1's column. After comments that start at or right of an inner scope, the comment's column closes the bodies right of it, as on Do not let a comment-only line close a layout scope #250, and the member follows the previous one. A scope anchored mid-line (the body after member x.A = on the same line) does not count as a scope at the member keyword's column: a NEWLINE there would put the member inside that body.

Every other line keeps the comment's column, exactly as on #250:

• a closing bracket, a leading ;, or a keyword the ordinary path decides layout for (then, else, with, end, …). That path runs before this one and cannot see past the comment.
• code at no open scope's column: between two levels, or right of the current one. Its meaning depends on the construct: an expression continues, but F# starts a new record field, member or match arm. The grammar gets several of these wrong without the comment too.
• a line after one that ends with ;. The ordinary path handles that with its own rule.
• ambiguous comments, where the rest of the parser may read the comments differently from the helper:
  • an empty comment (**), which the grammar can read as the ** operator where an expression may start. This doesn't apply before a member keyword, where (**) can only be a comment.
  • a (*) inside a comment, which scan_block_comment counts as an opening;
  • a " inside a comment. FSC skips strings and character literals in a comment; the comment token does not, so a (* or *) inside one ends or nests the comment elsewhere for the parser. Telling where a string ends needs escapes, character literals and triple quotes, so any quote is the test. The cost: a comment that merely quotes a word keeps Do not let a comment-only line close a layout scope #250's column.
  • a comment nested in another. The comment token ends a line-start comment at its first nested (* (on main too), so the parser reads the rest of it as code.

A line that closes several scopes is scanned once per DEDENT. While the open scopes' columns increase up the stack, each decision depends only on the line and on the scopes at or below the code's column, which the line's DEDENTs leave alone, so every scan of the line decides the same way. A version that decided from the innermost scope switched columns between scans.

That premise fails where a scope sits left of the one below it: a bracket scope (let x = ( with its items on the following lines, left of the let body's column), and the body of a function, fun or (fun that is anchored mid-line further right (let f = function with its arms at column 4; foo (fun x -> with the body at the outer column). There, an early scan finds no scope at the code's column and pops with the comment's column; a later scan, with that scope gone, takes the code's. The ; rule, too, holds for the line's first DEDENT only, because that token consumes the ;. I traced every decision over the 29,289 .fs probes below: 13,544 lines are scanned more than once, and in 89 files the scans of one line decide differently (67 function or fun bodies, 3 bracket scopes, 19 after a ;). None of the 89 gains an ERROR or MISSING node against #250, 32 lose some and 19 parse exactly as on #250; the trees of the rest differ from #250's at equal error counts, not always for the better.

The condition for taking this path is unchanged from #250. In particular, it is not taken after a directive line (#if, #else, #endif), which the DEDENT has to precede.

Tests

There are 35 new tests in test/corpus/scoping.txt. FCS (Fantomas.FCS 7.0.5) accepts every input, and I checked each expected tree's structure against FCS's AST.

On #250, 16 of the 35 parse to a wrong tree with no ERROR or MISSING node:

• code at the class member column and at the function body column;
• a member and a statement on the closing line of a multi-line comment;
• a member after two comments;
• a member that closes the previous member's body;
• + 2, and + one column left of the body, which continue;
• (), a new statement (the helper consumed its ();
• do at the column of a body that starts mid-line (let x = 1), which FCS makes that body's next item;
• foo+bar at the body column, a new statement;
• the word operator or, which continues;
• oré 2 and order 2 at the body column, new statements whose first word only starts like or;
• a member, and each of abstract, default, override, static member and val, after an empty comment (**), which Do not let a comment-only line close a layout scope #250 reads as the ** operator.

9 have an ERROR on #250:

• a match arm, and one whose pattern starts with a non-ASCII letter (|é -> 2);
• |> string, || b and / 2 at the body column;
• |> one column left;
• code that closes a nested let body;
• a list item at the list's item column;
• a record field left of the first field. FCS reads it as continuing the first field's value (A = 1 B = 2 is A = ((1 B) = 2)), and so does this change.

10 parse exactly as on #250. They are regression tests for the lines that keep the comment's column:

• a closing ), and a closing ] after an empty comment;
• end closing a class;
• a leading ;;
• a member right of the class column;
• a member keyword at the previous member body's column, and right of it, which FCS reads as the class's next member;
• a member keyword at a multi-line body's column after a comment at the class member column, and after one between that column and the body's;
• a match arm right of the arms after a comment at the arms' column, which FCS reads as the next arm.

All but nine of the trees also match the same file with its comments replaced by spaces. The nine are the member right of the class column, the four member-keyword tests at or right of a body's column, the record field, oré, |é and the arm right of the arms. In those layouts, the grammar misreads the file without the comment too (for oré and |é, the ordinary path reads or and | before a non-ASCII letter as operators).

Verification

Probes. The review of this change produced probe inputs in several rounds. FCS accepts 49,614 distinct ones without diagnostics: 46,856 .fs and 2,758 .fsi, each .fsi parsed with the signature grammar. ERROR and MISSING nodes are counted exactly, by query. Against #250:

	.fs	.fsi
unchanged	37,190	2,444
erroneous on #250, clean here	5,703	168
clean on #250, erroneous here	45	0
changed, clean on both	3,746	34
changed, erroneous on both	172	112

73 gain ERROR or MISSING nodes, the 45 above included. Every one is in one of these classes, and in each the grammar fails on the same layout without the comment: for all but four, the file with its comments replaced by spaces also errs or parses to the same tree. The four are oré 2 in the first class, which the ordinary path reads as or.

• 40: code at the column of a property accessor's scope (with get () = 1 / and set … / (* c *) ignore 1, with the code at the get's column). F# continues the accessor's body there. The grammar cannot take a separator at that scope, so the type gains ERRORs; Do not let a comment-only line close a layout scope #250 closed the type instead, with no ERROR. 38 of these are clean on Do not let a comment-only line close a layout scope #250.
• 16: | _ at the column of the previous arm's body, already erroneous on Do not let a comment-only line close a layout scope #250 (under "Not fixed").
• 10 .fsi: member A : int in a type signature, which the signature grammar cannot parse with or without the comment. Error recovery differs from Do not let a comment-only line close a layout scope #250's.
• 4: interface I with with no members, 2: extern in a class, 1: .. at an array item's column. The grammar errs on these without the comment; Do not let a comment-only line close a layout scope #250 parsed the line outside the type or array without an ERROR.

These counts are relative to this probe set. Other shapes can reach other grammar limits through the code's column.

Structure. I counted members (including abstract, val and member val), bindings (including let!), match clauses and record fields in FCS's AST and in both trees, for each changed .fs input without #if (FCS parses one branch of an #if, tree-sitter both). Of 7,409: 2,751 move closer to FCS, 4,457 keep the same distance, and 201 move farther:

• 164 are the accessor class above;
• 23 are records after an undented line (- 2 or "s" left of the fields). FCS reads the next field into the previous field's value; this change starts a new field, as the file without the comment does in 22 of them, and Do not let a comment-only line close a layout scope #250 had an ERROR on each;
• 11 are the | _ class;
• 3 are an artefact of the count: FCS represents member this.Item with get (i) = i as GetSetMember, which the count does not include.

A count cannot see a wrong tree with right counts, and the probes are not exhaustive. The last review round also checked the first member's body in 4,374 grid inputs: on all of them this change parses as #250 does, except 1,422 in the multi-line member body class under "Not fixed", which parse as the file without the comment does, with equal or fewer errors.

Against the previous revision. An earlier version took the code's column in more places: it read a quote in a comment as ambiguous only by quote parity and a nested comment not at all, and it took the code's column for a leading ; and for a member keyword after comments at any scope. On the same 49,614 probes, against #250, it made 9,017 inputs clean (this change: 5,871), but 167 went from clean to erroneous (45), 414 gained ERROR or MISSING nodes (73, all in the classes above), and 1,928 moved farther from FCS's counts (201). This version keeps #250's column wherever that earlier one regressed; the price is the fixes it forgoes, under "Not fixed".

Mutants. The suite kills 34 of 55 mutants of this decision:

• layout from the comment's column; the code's column taken one column early;
• no open-scope check, and bracket scopes ignored by it; all code skipping scopes anchored mid-line;
• each member-keyword rule: the comment's column never kept, kept after any comment, kept only on a scope exactly, or kept on the outermost scope too; each of the six member keywords dropped;
• an empty comment before a member keyword treated as ambiguous; nested comments not counted;
• / and ( classified after the helper consumed them; an operator read after an ordinary word; a word ended at a non-ASCII letter; a word starting with or read as or; | before a non-ASCII letter read as an operator;
• no keyword rule, and end dropped from it; no closing-bracket rule; no leading-; rule; no or; no | operators; no infix operator; an operator left of the scope dedenting;
• NEWLINE never, or also for an operator;
• no #if guard.

21 survive. I ran each one that changes anything over the 31,517 probes of the earlier rounds, on the revision before the mid-line skip was restored (restoring it changes no error count on the 49,614 probes). For comparison, that revision against #250: 45 clean→erroneous, 73 with more ERROR or MISSING, 4,366 erroneous→clean, and 1,537 closer to / 201 farther from FCS's counts.

• A scope at or right of the column instead of exactly. 159 clean→erroneous, 189 with more, 395 farther. A line between two levels would also DEDENT with the code's column and fall back to the comment's in a later scan.
• No early exit below the column (search past a shallower scope). The same errors, 62 more erroneous→clean, 6 more closer and 6 more farther. Kept: the decision would then depend on a scope beneath one the line's DEDENTs may pop, which is the instability this design avoids.
• No ambiguity for a " in a comment. 10 more clean→erroneous, 226 more with more errors, 225 more farther; but 1,113 more erroneous→clean and 369 more closer. The probes quote words in comments often; a test that follows FSC's string and character-literal lexing inside comments would recover most of those, and is left for a separate change.
• No ambiguity for a nested comment. No more clean→erroneous, but 117 more with more errors and 16 more farther, against 40 more erroneous→clean and 31 more closer.
• No ambiguity for a (*) in a comment. No more clean→erroneous and none more erroneous→clean, but 10 more with more errors; 64 more closer and 10 more farther.
• No ambiguity for an empty comment. 7 more clean→erroneous and 7 more with more errors (three of them the attribute case under "Not fixed"); but 320 more erroneous→clean, 150 more closer and 17 more farther. Where the ** reading is possible depends on what follows the comment, which the helper does not parse.
• A nested empty comment not seen as empty. Equivalent: a nested comment keeps the comment's column anyway.
• The member keyword's mid-line skip. Without it, a member keyword at the column of a one-line member body, after comments that start left of every inner scope, takes a NEWLINE inside that body: 336 grid inputs of the last review round then put the member's text into the previous member's body, which Do not let a comment-only line close a layout scope #250 keeps intact. With it, those lines keep Do not let a comment-only line close a layout scope #250's tree, which reads the member outside the type, so a test would record a tree FCS disagrees with.
• No ; rule. No more clean→erroneous, one more with more errors (a | _ arm), 17 more erroneous→clean, 8 more closer and 6 more farther. Kept because the ordinary path gives a line after a ; its own reading (+ 2 there is a prefix application, not a continuation), which this path would bypass.
• The nine layout keywords other than end, dropped together. One more clean→erroneous and 3 more with more errors; 44 more erroneous→clean, 24 more closer and 20 more farther. The ordinary path lays these keywords out by rules this path does not repeat, so they keep Do not let a comment-only line close a layout scope #250's column. Among the probes that parse without errors here, dropping one keyword changes 59 trees, and 58 keep the same distance to FCS's counts; the other, for with, moves closer. The probes offer no test that pins one with a tree FCS agrees with.
• No rule for |] and |}, and end-of-input read after the classification. No probe tells either apart.
• CR not ending a comment-only line. It changes the tree of 82 FCS-valid probes, all CRLF, but no test can carry CRLF: .gitattributes normalises the corpus to LF.

The rules without a killing test are the survivors above. No test pins the fallbacks for a ", a nested comment, a (*) or an empty comment. Among the probes that parse without errors here, none tells one of them apart where this change matches FCS's counts and the mutant does not, so a pinning test would record an erroneous tree, or one FCS disagrees with.

Corpus

Measured against #250, which this is stacked on, and against main:

• check:baseline (.fs): ImpliedSignatureHashTests.fs goes from 34 to 0, and no file parses worse.
  • The total goes from 4447 to 4413; it is 4458 on main.
  • The baseline is updated. fsyaccast.fs's compiler-dependent entry stays at 6, as in Do not let a comment-only line close a layout scope #250.
• check:invalid: all 346 recorded errors are still found.
• .fsx (770 files, which the baseline script doesn't sweep): no change from Do not let a comment-only line close a layout scope #250, 2181. Against main, the two copies of libtest.fsx and comprehensions-hw are worse, as Do not let a comment-only line close a layout scope #250 describes, and members/basics-hw is better.
• .fsi (348 files, each parsed with the signature grammar): 9830 on all three.
• E_OperatorOverloading01.fs, which has a member right of the class column after a comment, parses exactly as on Do not let a comment-only line close a layout scope #250.
• parser.c does not change; this is a scanner-only fix.

Not fixed here

Each of these is wrong on #250 as well. Where the recovery differs from #250's, the item says so.

• A keyword after a comment at the scope's column ((* c *) else 2 under if … then) is wrong on main and here alike.
• Code at no open scope's column after a comment. Its meaning depends on the construct, and the grammar misreads several of these layouts without the comment too:
  • a line right of the current scope, which continues an expression. Declining there instead, as the file without the comment does, was measured on the probes above: 379 inputs then go from clean to erroneous, most of them right of a property accessor's scope, against this change's 45;
  • a record field right of the first field, which FCS makes a new field;
  • a line between two open levels after a nested block, which needs a separator.
• Code at the column of a property accessor's scope, and the other grammar limits under Verification. The code's column now reaches them; Do not let a comment-only line close a layout scope #250's comment column closed the type before them.
• A member keyword after comments that start left of every inner scope, at a multi-line member body's column. It is read inside that body, as without the comment; FCS ends the body there. Do not let a comment-only line close a layout scope #250 read it outside the type.
• A member keyword after comments that start at or right of an enclosing scope that is not the member list: a module's body scope left of a class's member column, an interface implementation's or an object expression's enclosing type. The comment's column closes the class or the implementation, as on Do not let a comment-only line close a layout scope #250. The scanner cannot tell a member list's scope from a member body's or a module's; the rule that would (take the code's column only below the innermost scope) changes its answer between the line's DEDENT scans.
• A leading ; after a comment keeps the comment's column, as on Do not let a comment-only line close a layout scope #250. That is right for a ; ending an interface's members and wrong for a ; separating list, array or sequence items at the code's column, which F# continues.
• A member keyword at a one-line member body's column after comments that start left of every inner scope keeps Do not let a comment-only line close a layout scope #250's reading, outside the type.
• Ambiguous comments (an empty one before anything but a member keyword, a (*), a " or a nested comment in one) keep Do not let a comment-only line close a layout scope #250's column, so the fix does not reach them. For the empty comment that is the price of (**) [<Obsolete>] before a module's let, where the grammar would read the (**) as the ** operator and gain ERRORs; Mutants above gives both sides.
• A line after one that ends with ;.
• | _ at the column of the previous arm's body (on its own line, or after -> on the arm's line). FCS starts a new arm. The grammar reads the line as part of that body even without the comment. Here, error recovery gains one ERROR, and it can also take the declaration after the match, which Do not let a comment-only line close a layout scope #250 kept.
• A multi-line comment that opens exactly at the scope's column. This path needs the line to start left of the current scope.
• A comment-only line before the first line of an indented block (let f () = / (* note *) / 1). The INDENT decision still anchors the block at the comment, also when code follows the comment there. As in Do not let a comment-only line close a layout scope #250, this needs its own change.
• A block comment that starts mid-line is consumed as an extra. If it spans lines, the code on its closing line is laid out from the line the comment started on. This change only concerns lines that start with a comment.
• Columns are code points. A character outside the Basic Multilingual Plane (an emoji) in a comment counts as one column here and two for FSC, which counts UTF-16 units. A tab counts as 8 columns in the line's indentation and as 1 in the code's column; F# rejects tabs without #indent "off".

[pcshrosbree]

This PR is one of a set; #255 maps them all, with their dependencies and merge order.