[pull] master from GaijinEntertainment:master#982
Merged
Conversation
New rules
- PERF018: `for (i in range(length(arr))) { ... arr[i] ... }` when `i` is
only an index of arr -- suggests `for (c in arr)`. AST-driven detection
via for-source / for-variable / for-body callbacks + ExprAt qualifying-
index counter (`in_qualifying_idx`). Conservative: only bare-Var arr
resolves; field paths and arithmetic on `i` disqualify.
- STYLE020: `from_JV(v, type<T>, defV)` resolving to one of json_boost's
scalar overloads -- suggests `v ?? defV` (operator ?? lives at lines
85-140 of json_boost). Skips inside json_boost itself + generic
instantiations of its templates.
- STYLE021: a `var v : table<string; JsonValue?>` followed by 2+
contiguous `v |> insert(<const>, <any>)` calls -- suggests
`var v = JV((k1=..., k2=...))` (named-tuple JV dispatch at
json_boost.das:638).
STYLE005 rewrite
- Was: gated behind `--postfix-conditionals`, fired on multi-line braced
forms, skipped same-line forms (which it called "already postfix").
- Is now: flag `if (cond) { return/break/continue }` (braces around a
single early-exit terminator). Leaves braceless `if (cond) return` and
postfix `return X if (cond)` alone. Discriminator is AST-only:
`if_true.at != inner_terminator.at` <=> source had braces (synthetic
blocks share LineInfo with the inner statement).
- Drop `postfix_conditionals` from `style_lint` / `style_lint_collect`
public API + the matching CLI flag in utils/lint/main.das.
Lint runner
- utils/lint/main.das: treat "missing prerequisite" compile errors
(e.g. daspkg-installed `anthropic/anthropic`) as SKIP rather than
FAIL -- those files are out-of-tree from a stock daslang build's POV.
- utils/mcp/subtools/lint_tool.das: drop the now-removed
`postfix_conditionals` arg from style_lint_collect calls.
Real fixes surfaced by the new STYLE005
- daslib/builtin.das: `clone` overloads -- drop the brace-wrap around
the early-return.
- utils/mcp/subtools/lint_tool.das: postfix-form rewrites for 5
single-terminator guards.
- utils/requirefix/main.das: `require fio` -> `require daslib/fio` (was
silently broken under the lint runner's empty-project file access).
Tests
- utils/lint/tests/perf018_index_only_for_loop.das (expect 31208:3) --
bad: 3 index-only patterns; good: index-used, two-arrays, sliding-
window, for-each.
- utils/lint/tests/style005_braced_terminator.das (expect 31209:5) --
replaces style005_postfix_conditional.das. Bad: braced return/break/
continue, one-line braced form. Good: braceless, postfix,
multi-statement block.
- utils/lint/tests/style020_from_jv_to_coalesce.das (expect 31209:6) --
scalar overloads flagged; vector/struct/already-`??` forms silent.
Follow-up branch will land utils/fix-lint-errors/ (auto-fixer for the
mechanical rules) and the daslib + utils sweep.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New tool: utils/fix-lint-errors/main.das.
Pipeline (matches utils/lint/main.das structure):
- scan_das_files() walks input paths.
- collect_edits() compile_file's each .das, runs FixerVisitor over the AST,
collects SourceEdit records.
- apply_edits_to_file() sorts edits by descending end position and applies
them as line-range splices (so earlier offsets stay valid as we go).
FixerVisitor — one rule for now:
- STYLE005: `if (cond) { <terminator> }` → `if (cond) <terminator>`.
AST shape gate: ifte.if_true is ExprBlock, single-statement body, the
inner is ExprReturn/Break/Continue, AND `blk.at != inner.at` (synthetic
Blocks share LineInfo with the inner stmt). Replacement text is the
inner-of-braces source, trimmed — captures keyword+subexpr+any `;`
cleanly, where extract_text(inner.at) would only get the keyword.
Coordinate convention recorded in extract_block_inner's docstring:
LineInfo.column is 0-based, last_column exclusive 0-based. Got this wrong
on first pass — the `{` was off by one and the rewrite emitted broken
output like `retur}`. Now correct.
CLI: --dry-run prints would-be edits without writing, --quiet suppresses
PASS lines, --help prints usage. Positional args are files or dirs.
Smoke-tested on synthetic probes covering braceless, postfix-desugared,
and multi-line braced forms. Real daslib/utils sweep is the next commit
(this branch).
Adds four mechanical rewrite rules to the auto-fixer:
PERF013 a += 1 / a -= 1 -> a++ / a--
PERF017 length(x) == 0 / != 0 -> empty(x) / !empty(x)
STYLE017 if (cond) return true; else return false -> return cond/!cond
(both forms: in-place else-branch and adjacent return)
STYLE018 b == true / b != false -> b ; b == false / b != true -> !b
Each rule shares the same Edit pipeline (visitor override pushes a
SourceEdit; apply_edits writes them back from end-of-file). New
infrastructure:
- extract_source / extract_source_at multi-line LineInfo -> bytes
- leftmost_pos_of / rightmost_pos_of chains walk into root/leaf since
ExprField.at covers only `.` and ExprAt.at covers only `[`
- source_span_of adds atEnclosure handling for ExprCall
- is_simple_subject gate for rules that need clean source spans
- apply_edits_to_file overlap filter drops edits subsumed by a larger
one (e.g. STYLE005 inside STYLE017(b) span)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds two more mechanical rewrite rules to the auto-fixer:
PERF015 a > b ? a : b -> max(a, b) (and min variants)
PERF016 x < 0 ? -x : x -> abs(x) (and other ternary abs forms)
New infrastructure:
- extract_expr_source — wraps leftmost+rightmost+extract_source_at into one
call, used for splicing operand text into replacements
- is_const_zero / is_neg_of / expr_describe_equal — mirror the matching
helpers in daslib/perf_lint.das so the fixer detects the same patterns
- rightmost_pos_of now recurses into ExprOp1 (-x's right end lives in
subexpr) and ExprRef2Value (typer wrappers don't shift source)
- lo_min / hi_max — guarded min/max for source spans, with sentinel-0
handling so unset bounds don't accidentally win
The PERF015/16 span computation takes min(leftmost) over both cmp.left and
cmp.right because the typer canonicalizes Yoda forms (0 > x becomes x < 0
in AST), so cmp.left isn't always the source-left operand.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Improvements based on what real daslib code surfaced: 1. visit_with_generics match the lint visitor's coverage. The previous `visit()` skipped uninstantiated generics, missing ~30% of hits. 2. in_target_file gate visit_with_generics walks AST that may live in other files (builtin instantiations triggered from us). Without the gate, edits with cross-file coordinates corrupted the target file. 3. Chain helpers handle auto-deref leftmost_pos_of now recurses past ExprPtr2Ref and ExprRef2Value. `fun.moreFlags.isTemplate` was losing its `fun` prefix because the typer inserts ExprPtr2Ref between fun and .moreFlags whose `.at` is the dot, not fun. 4. rightmost_pos_of handles ExprOp1 `-x`'s right end lives in subexpr. Without this, PERF016 `... ? -x : x` truncated to `abs(x)x`. 5. is_simple_subject recurses chains rejects ExprPtr2Ref anywhere in the chain since parens around `(*pv)` aren't in AST. PERF013/PERF017 /STYLE018 bail rather than corrupting source. 6. STYLE017(a) disabled match blocks parse as nested ExprIfThenElse chains where the wildcard arm's body becomes the innermost else, making them indistinguishable from real if/else. The lint reports these as STYLE017 too; auto-fixing them produced broken match code. Hand-fix the lint reports. 7. file_requires_math gate PERF015/PERF016 rewrite to math::min/max/ abs. Files without `require math` would fail to compile post-rewrite. Source-line scan determines applicability. 8. Auto-revert safety net after writing a file, try_compile re-reads it. If compile fails, the original is restored and reported as REVERT instead of corrupting state. This catches the edge cases the per-rule gates miss; daslib sweep went from 60 cascade failures to 8 cleanly-reverted files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drives the daslib bulk sweep from 8 cleanly-reverted files down to 0 by
diagnosing each break in turn and fixing the underlying span/shape bug,
rather than relying on the auto-revert safety net.
Bugs fixed:
1. Implicit-self field access — `length(field)` inside a class method
gets parsed as ExprField(value=ExprVar(self), name=...) where the
field's `.at` already covers the source field name (NOT just the `.`
separator). is_implicit_self_field discriminator + at-aware leftmost
/rightmost. Was producing `!empty(exprForTerminator) > 0) {))`.
2. `[macro_function]` skip — these set `fn.moreFlags.macroFunction` not
`isTemplate`. Bodies contain macro-substituted AST whose source
positions don't reliably point to user-written source. Bail.
3. Source-text sanity gate — extract_expr_source rejects extractions
that start with punctuation (`.`, `]`, `,`, `)`, `}`). Catches any
remaining chain-helper miss without corrupting the file.
4. ExprOp1/Op2/Op3 in left/rightmost — binary cond chains like
`A || B || C` need recursion into ExprOp2 operands; `-x` and `!x`
need rightmost to recurse into subexpr. `-x` was producing `abs(x)x`;
`A || B || C` produced `|| pt.annotation.name != "TypeDecl" || ...`.
5. Pipe-form calls — `arr |> length` parses to ExprCall whose name is
AT `length` (later in source) and args[0] precedes it. leftmost
takes min(call.at, arg[0]'s leftmost); rightmost takes max over
atEnclosure, call.at, and all args.
6. Parenthesized comparisons — `(a < b) ? a : b` had the closing `)`
swallowed by PERF015/16 spans. is_parenthesized_cmp peeks the source
line for `)` right after cmp.right and bails.
7. Property-method calls — `obj.foo` (with `isProperty`) parses to
ExprCall name=".foo" or "`.foo" whose source extent depends on
hidden arg positions. cond_uses_property_call walks the cond tree
and STYLE017 bails if any property call is involved.
8. STYLE005 multi-statement sanity — extracted inner text MUST start
with `return`/`break`/`continue`. Defends against `visit_with_generics`
surfacing optimized AST shapes whose block-length count doesn't
match user source.
9. Bonus: --no-verify flag for debugging — disables the auto-revert
safety net so the broken-rewrite output can be inspected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applied utils/fix-lint-errors over daslib/ — every change is the
deterministic mechanical equivalent the lint rule already prescribes.
All 79 files compile cleanly post-rewrite; no compile-verify reverts.
Breakdown (1131 edits):
STYLE005 — `if (cond) { return X }` -> `if (cond) return X` (most hits)
PERF017 — `length(x) == 0 / != 0` -> `empty(x) / !empty(x)`
PERF013 — `a += 1 / -= 1` -> `a++ / a--`
PERF015 — ternary min/max -> `min(a, b) / max(a, b)`
PERF016 — ternary abs -> `abs(x)`
STYLE017 — `if (c) return true; return false;` -> `return c` (or !c)
STYLE018 — `b == true / != false` -> b / !b
Net: −1987 lines (lots of brace-wrapping noise removed). Lint count
across daslib dropped from ~1500 to 449. The remaining 449 are
structural rewrites the auto-fixer correctly skips:
STYLE016 (112) — adjacent guards with identical early-exit; needs
`||`-combination, has cross-statement scope
STYLE005 (181) — cases where the body's source extent isn't safely
reliable (property-method chains, generics)
PERF017 (78) — same — `length()` involves unsafe-to-rewrite chains
PERF014 (10) — closed-interval char-class ranges (not auto-fixable
without a small AST replanting)
remainder — STYLE011/013, PERF006/007/012/018: each rule is
structural and warrants hand-curation per call site.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `from_JV(v, type<T>, defV)` overloads in `daslib/json_boost` are `[template(ent)]` generics, so a user call goes through two levels of template instantiation before the visitor sees it. expr.func.name is mangled and the `ent` witness is stripped from arguments — so the original check (`expr.func.name == "from_JV"` and `arguments[1]._type` for the type witness) never matched. STYLE020 silently fired 0 times since it was introduced. Walk `fromGeneric` to the root and match on the root's name/module instead. Use `expr._type` (the call result type) for the supported-scalar check — robust whether the visitor sees the pre- or post-instantiation shape. Lint test fixtures (style005 always-on bleed): - style017_bool_return: refactor `good_same_value` / `good_compute` to braceless form so STYLE005 doesn't also fire on the STYLE017 negatives. - style016_adjacent_guards: refactor bad-case bodies to braceless so the fixture exercises STYLE016 alone (STYLE005 has its own fixture). The `bad_else_chain` keeps the outer braces (for the elif AST shape) but the elif body switches to braceless. All 21 lint tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The post-rewrite verification compile was running with `ignore_shared_modules = true`, `no_optimizations = true`, and `no_infer_time_folding = true`. These strict settings produced false-positive reverts on files that transitively require dastest (UnrollMacro needs `range(const)`, which needs const-folding) or daspkg packages like `sqlite/sqlite_boost` (which need shared modules loaded). Drop the three flags. Keep `export_all = true` (matches the lint pass) and `lint_check = false` (the lint pass already ran and reported a clean compile, so re-running lint is wasteful). The collect_edits pre-pass still runs with the original strict settings; only the post-rewrite verification is relaxed. This change recovers ~10 files per utils/ sweep that previously auto-reverted despite producing clean compiles (e.g. utils/benchctl/*). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three corruption fixes surfaced by the utils/ sweep: 1) Skip `fn.flags.generated`. Functions synthesized by struct annotations (e.g. `[CommandLineArgs]` → `parse_args`, `get_command_info`) carry the struct's LineInfo throughout their body. A `length(args) == 0` inside the synthesized parse_args body would rewrite to a span starting at the struct definition, eating ~889 lines on utils/daspkg/commands.das. 2) Guard STYLE017(b) against postfix-desugared `return X if (cond)`. The parser desugars postfix conditional returns into an ExprIfThenElse whose `.at` points at the original `return`, not `if`. STYLE017(b)'s span computation would then clobber the leading `return X` token. Detect by peeking the source line: bail if it does not start (after whitespace, optional `}`) with `if` or `elif`. 3) Guard STYLE017(b) against conds containing the typer-lowered form of `x is T` (`x.__rtti == "..."`). The synthesized ExprConstString carries an `at` that no longer covers the original type-name source span, so rightmost_pos_of stops at `is` and the rewrite drops the trailing typename (`|| e is ExprOp1)` → `|| e is)`). Self-apply on utils/fix-lint-errors/main.das now produces 32 clean edits and a passing post-rewrite verify; full utils/ sweep reverts drop to just the find-dupe `anthropic/` daspkg-missing pair (2 fails). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applied utils/fix-lint-errors to utils/. The fixer's three new guards (macro-generated skip, postfix-if discriminator, typer-lowered-is bail) land 0 auto-reverts. The only skipped files are utils/find-dupe/judge_live and utils/find-dupe/main, which require the daspkg-only `anthropic/` package; users who install it via `daspkg install --root utils/find-dupe` can re-run the fixer to pick those up. Net -1517 LOC across utils/. Lint count drops from 1154 → 347. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cosmetic whitespace normalization from `mcp__daslang__format_file` to keep the test fixture canonical-formatted; CI-fmt parity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- CLAUDE.md: add a "Braceless early-exit" bullet under Error handling pointing readers to STYLE005, and four new rows in the idiomatic-forms table (STYLE005 / PERF018 / STYLE020 / STYLE021). - doc/source/reference/language/lint.rst: STYLE005 section rewritten (always-on, braceless preferred), PERF018 section added after PERF017, STYLE020 + STYLE021 sections added after STYLE019. Drop the stale `--postfix-conditionals` CLI flag from the standalone-utility options. - skills/perf_lint.md: PERF018 row appended to the rules table. - skills/style_lint.md: STYLE005 row description rewritten; STYLE020 + STYLE021 rows added; Public API drops the `postfix_conditionals` parameter (removed when STYLE005 became always-on). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…and-fixer lint: PERF018/STYLE005/STYLE020/STYLE021 + auto-fixer + daslib+utils sweep
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )