Add block-scalar-chomping rule#286
Conversation
…ator (#257) Off-by-default, ryl-only (TOML) rule flagging literal/folded block scalars that lack an explicit chomping indicator (`-`/`+`); `|2` (indentation-only) is flagged. Enumerates block scalars via granit scanner tokens and recovers the header as the nearest marker-bearing line above the content, splitting lines on granit's YAML 1.2 break set (`\r\n`/`\r`/`\n`) so it is CR-aware like the other parser-based rules. Empty/blank-only scalars are skipped (granit gives them no stable header position). No safe `--fix` (YAML has no explicit clip indicator). Also tightens the `unicode-line-breaks` line-counting doc. Follow-up #284 tracks unifying the byte-scanning rules onto the same YAML 1.2 break set.
Deploying ryl-docs with
|
| Latest commit: |
fc4a4be
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://caee09c3.ryl-docs.pages.dev |
| Branch Preview URL: | https://257-block-scalar-chomping.ryl-docs.pages.dev |
Test Coverage (nextest + cargo-llvm-cov)Missed Lines (per file) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fa60599ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let text = strip_trailing_comment_preserving_quotes(lines[line_no - 1]); | ||
| block_scalar_marker_index(text).map(|idx| (line_no, text, idx)) |
There was a problem hiding this comment.
Preserve non-comment hash characters while locating headers
When the enabled rule checks a valid block scalar whose header contains an unquoted # that is not preceded by whitespace, such as a#b: |\n body\n or a verbatim tag URI with a fragment, this helper truncates the header at the #. The reverse search then cannot find the actual marker and the .expect(...) below panics, crashing the CLI on valid YAML instead of reporting the missing chomping indicator. Only strip # when it begins a YAML comment, or avoid comment stripping while recovering scanner-confirmed headers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fa60599ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let text = strip_trailing_comment_preserving_quotes(line); | ||
| block_scalar_marker_index(text) |
There was a problem hiding this comment.
Preserve
# inside verbatim tags when locating headers
When this rule is enabled, a valid header such as value: !<tag:example.com,2000:app/foo#bar> | can panic the CLI. Verbatim tag URIs may contain #, but strip_trailing_comment_preserving_quotes treats every unquoted # as the start of a comment, truncating the line before the block-scalar marker; header_marker then finds no marker and reaches its expect. Only strip a # that is actually a YAML comment, or locate the header using scanner spans that preserve tag fragments.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 268780578b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| block_scalar_marker_index(text).map(|idx| (line_no, text, idx)) | ||
| }) | ||
| }) | ||
| .expect("a block scalar has a marker-bearing header") |
There was a problem hiding this comment.
Avoid panicking on plain keys containing
! or &
When this rule is enabled, valid YAML such as a !foo: |\n body\n crashes here instead of reporting the bare header. block_scalar_marker_index treats the whitespace-delimited !foo: token as a node property, walks past it, and rejects the actual header because the remaining prefix ends in a; the backward search therefore returns None and this expect panics. Plain-scalar keys may contain ! or & after their first character (the example is also accepted by yamllint), so header recovery must distinguish those key fragments from actual node properties.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
block-scalar-chompingrule from New rule:block-scalar-chomping— require explicit chomping indicator #257-) or keep (+) indicator on every literal or folded block scalar, including indentation-only, empty, and blank-only scalarsWhy
A bare
|or>silently selects YAML's clip chomping behavior. Requiring an explicit indicator makes the intended trailing-newline behavior visible and avoids accidental value changes.During review, empty and blank-only scalars were found to be skipped even though #257 requires every block scalar to be checked. Header recovery now handles granit's different token positions for those scalars while avoiding false header matches when the first content line itself looks like
|or>.User impact
The rule is disabled by default and is configured only through TOML because yamllint has no equivalent rule. It has no safe automatic fix because YAML has no explicit clip indicator; choosing strip or keep changes the resolved value.
Validation
prek run --all-files./scripts/coverage-missing.sh(Coverage OK: no uncovered regions.)PROPTEST_CASES=512000 cargo test --release --test property_checkuv run scripts/source_size.py --compare-to mainCloses #257
Part of #261