-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
@Teakowa
Teakowa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Three required fixes before this matches #309:
crates/wright-analyzer/src/declarative.rs/docs/specs/SPEC-309-extensible-lint-rules.md: external rule YAML currently requiresevidenceanddefault-severity, while #309 explicitly keeps evidence classification Wright/owner-backed and project severity outside rule definitions. Project config also exposes onlywarning/info, not the approvedoff/warn/errorpolicy. Align the public rule/config contract with the Issue AC rather than freezing the conflicting surface in an accepted spec.crates/wright-analyzer/src/declarative.rs: the matcher implements positional args plus exact literal equality only. #309 requires direct</<=/>/>=comparisons and localized/canonical parameter names, with representative complex-rule validation. Implement and verify those approved capabilities before treating the grammar as stable.crates/wright-analyzer/src/declarative.rs::condition_values: scoped condition matching walksfacts.actions()for the whole Workshop rule, so ascope: while/ifmatcher can be satisfied by sibling or unrelated nested conditions; the selectedifbranch condition is not scoped correctly either. Restrict condition candidates to the selected scope so unrelated conditions cannot produce findings.
@Teakowa
Teakowa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crates/wright-analyzer/src/declarative.rs::resolve_parameter still does not satisfy the previous localized/canonical parameter-name finding. _locale is unused and resolution only compares against CatalogEntry::params; with the pinned workshop-rs contract those are canonical parameter labels, not locale-specific aliases. The updated spec also narrows this to "canonical catalog labels", which conflicts with #309 rather than implementing its document-locale requirement. The new test only covers English duration, with no non-en-US/representative complex parameter case. Resolve parameter spellings through owner-backed locale data (adding the missing workshop-rs contract first if necessary) and verify canonical plus localized parameter authoring before treating the grammar as accepted.
e54-bot
commented
Sep 10, 2026
Addressed the latest review in f2ac79f. Parameter spellings now resolve only through the owner-backed workshop-rs catalog API: canonical labels and localized zh-CN labels are both covered, with no Wright-side translation table or normalization fallback. The owner contract is proposed in workshop-rs PR #176; this branch pins that commit until the owner change is released. Local verification: cargo fmt --all -- --check; cargo clippy --workspace --all-targets --all-features -- -D warnings; cargo test --workspace --all-targets --all-features (all passed; the two corpus-dependent tests remain intentionally ignored).
@Teakowa
Teakowa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cargo.toml: the locale fix now correctly consumes an owner API, but it pins an unreleased commit from open workshop-rs#176 as a second workshop-rs dependency alongside =0.1.18. This skips the required owner -> contract/tests -> release -> consumer integration sequence and leaves Wright coupled to a transient post-v0.3.4 commit. Merge/release the owner capability first, then consume the released workshop-rs version here as the normal dependency rather than a parallel git-pinned catalog crate.
e54-bot
commented
Sep 11, 2026
Owner review corrections are now pushed to workshop-rs PR #176 in 46e27e1: catalog dataset version 0.1.4, refreshed digest, and explicit provenance for the initial Wait zh-CN parameter labels. Per the review's required owner -> release -> consumer sequence, Wright is intentionally not changed again until #176 is merged and a released workshop-rs version containing resolve_param is available; then the temporary git-pinned catalog dependency will be replaced with the normal released dependency. Wright's existing 18 hosted checks remain green.
e54-bot
commented
Sep 11, 2026
Follow-up: workshop-rs PR #176 now includes the reviewer-requested correction in commit 260c496, using the evidenced Wait zh-CN parameter spellings 时间 and 等待行为. All 4 owner hosted checks pass. Wright remains unchanged pending reviewer reapproval and the required owner merge/release step.
e54-bot
commented
Sep 11, 2026
Owner capability is now released: workshop-rs 0.3.6 includes resolve_param after workshop-rs PR #176 was merged. Wright commit 8ff16bc now consumes the released registry package instead of the temporary git-pinned catalog dependency, updates Cargo.lock, and aligns the localized Wait test with the evidenced spelling 时间. Local fmt, clippy, full workspace tests, and diff validation pass.
Add canonical Workshop facts, local YAML rules, bounded project configuration, and shared CLI/tool metadata surfaces. Fixes #309
Expose canonical-evidence skips as additive machine-readable lint result data.
Align external rule metadata with Wright-owned evidence and project severity policy. Add catalog-backed named parameter and numeric comparison matching, and scope condition candidates to the selected rule scope. Refs #309
@Teakowa
Teakowa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
SemanticFactsand local YAML declarative lint rulesworkshop-rswright lintVerification
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --all-targets --all-featuresCloses #309