Repository navigation
fix(cli): tighten please-style check heuristics deferred from #36 - #42
Conversation
…ramework-reserved names Exempt Dart files only when the last stem segment names a known generator (g, freezed, mocks, gr, pb, ...), so a handwritten dotted name is judged. Treat Route/GET/loader/action etc. as framework-reserved only under routes/ or pages/ directories. Refs #37
… error-file rule The kotlin-error-outside-errors-file rule now ignores Kotlin Multiplatform test source sets. Refs #37
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 2 high |
🟢 Metrics 17 complexity · 0 duplication
Metric Results Complexity 17 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the style checker by expanding Rust test attribute detection to support conditionally applied attributes like #[cfg_attr(..., test)], updating Kotlin rules to ignore all test source sets ending in Test, and adding support for Yarn Plug'n'Play environments by skipping sgconfig.yml generation with a warning. Additionally, it refines TypeScript/TSX filename checks to restrict framework-reserved exports to route or page directories and expands Dart generator output exemptions to cover several new suffixes. As there are no review comments provided, I have no feedback to evaluate.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 12 files
Architecture diagram
sequenceDiagram
participant CLI as please-style check CLI
participant FS as File System
participant Config as sgconfig.yml
participant Extractor as Extract Rules
participant FilenameCheck as check-filenames
participant RustCheck as Rust check
participant KotlinCheck as Kotlin check
participant Init as init command
Note over CLI,FS: Runtime flow for please-style check
CLI->>FS: Read project files
CLI->>Extractor: Run ast-grep extraction
Extractor->>FS: Read rules from node_modules/@pleaseai/ast-grep-config
Note over FilenameCheck: TypeScript/Dart filename validation
FilenameCheck->>FS: Read source files
FilenameCheck->>FilenameCheck: Evaluate export names
alt TypeScript file in routes/ or pages/
FilenameCheck->>FilenameCheck: Check if export is framework-reserved
Note over FilenameCheck: Reserved: Route, GET, POST, loader, action...
FilenameCheck-->>CLI: Skip with notice
else TypeScript file elsewhere
FilenameCheck->>FilenameCheck: Check export name against filename
FilenameCheck-->>CLI: Report if mismatch
end
alt Dart file with generator suffix
FilenameCheck->>FilenameCheck: Check for .g, .freezed, .mocks, .gr, etc.
Note over FilenameCheck: Generated: *.g.dart, *.freezed.dart, *.mocks.dart...
FilenameCheck-->>CLI: Exempt from check
else Dart file with handwritten name
FilenameCheck->>FilenameCheck: Check stem against filename
FilenameCheck-->>CLI: Report if mismatch
end
Note over RustCheck: Rust test detection
RustCheck->>FS: Read Cargo metadata
RustCheck->>FS: Parse Rust files
RustCheck->>RustCheck: Extract #[cfg_attr(...)] attributes
alt cfg_attr with test argument
RustCheck->>RustCheck: Check predicate (test NOT in predicate)
RustCheck-->>CLI: Mark as test file
else test used as predicate or in predicate
RustCheck-->>CLI: Not counted as test
end
Note over KotlinCheck: Kotlin error file validation
KotlinCheck->>FS: Read Kotlin source files
KotlinCheck->>KotlinCheck: Check path pattern
alt Path matches **/src/*Test/**
Note over KotlinCheck: androidTest, commonTest, jvmTest, iosTest...
KotlinCheck-->>CLI: Exempt from error check
else Path is in main source set
KotlinCheck-->>CLI: Report if error outside errors file
end
Note over Init: Init command with Yarn PnP
Init->>FS: Check for .pnp.cjs or .pnp.js
alt Yarn PnP detected
Init->>Init: Warning: node_modules not available
Init-->>CLI: Report step skipped
else Normal setup
Init->>Config: Write sgconfig.yml
Init->>FS: Reference rules in node_modules
end
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
- Detect Yarn Plug'n'Play from a workspace package (.pnp.cjs at an enclosing root), unless the project has its own installed rules directory - Do not treat cfg_attr(…, test::helper) as a Rust test attribute - Document .pnp.js and the yarn install + init rerun step Refs #37
Deploying code-style with
|
| Latest commit: |
495764c
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
Document that an installed rules directory overrides an enclosing Plug'n'Play root. Refs #37
|
1 participant
Summary
Tightens the
please-style checkheuristics that were deferred from #36. The PR has four commits covering these five changes:check-filenames.ts): The blanket "dotted stem" exemption is narrowed to a generator-suffix list (g,freezed,mocks,gr,config,gen,chopper,mapper,gql,graphql,pb,pbenum,pbjson,pbserver,pbgrpc). Mockito*.mocks.dartand protobuf files are now exempt even though their headers don't matchGENERATED_HEADER. Handwritten dotted names such aswidgets.custom.dartare now judged.loader,action,Route, HTTP verbs) are reserved only for files under aroutes/orpages/directory. Elsewhere they are judged like any other export. Inside route directories the behavior is unchanged (withheld with a notice).test-attrextraction rule now recognizes#[cfg_attr(<pred>, test)]and#[cfg_attr(<pred>, tokio::test(…))]through an AST branch. Atestinside the predicate (cfg_attr(all(unix, test), ignore)) or as the predicate (cfg_attr(test, derive(Debug))) does not count. Known gap: nestedcfg_attr(a, cfg_attr(b, test))still under-reports.kotlin-error-outside-errors-fileignores**/src/*Test/**(AndroidandroidUnitTest, KMPcommonTest,jvmTest,iosTest,jsTest, …). An e2e test confirmscommonMainis still reported.appliers.ts): when.pnp.cjsor.pnp.jsexists,initdoes not writesgconfig.yml, because thenode_modules/...rules path would not exist. It prints an en/ko warning to setnodeLinker: node-modulesand reports the step as skipped.The READMEs (en/ko) and the Kotlin rule README are updated.
Related issue
Closes #37
Verification
packages/cli:bun test— 241 passpackages/ast-grep-config:ast-grep test— 11 rules pass; e2e — 3 passbun run buildandbun run lintpasstsc --noEmitonpackages/clipassesOut of scope (follow-ups)
.editorconfigapplier andplease-style check's rule resolution also assumenode_modulesunder PnP.jvmLayoutinlayouts.tsonly recognizessrc/main|test|testFixtures, so KMP source sets are not covered by the test-path check.Checklist
bun run build)bun run lint)BREAKING CHANGE:note is includedSummary by cubic
Tightens the
please-style checkheuristics deferred from #36, closing #37.g,freezed,mocks,gr,config,gen,chopper,mapper,gql,graphql,pb,pbenum,pbjson,pbserver,pbgrpc), so handwritten dotted names likewidgets.custom.dartare now judged.Route, HTTP verbs,loader,action) are withheld only underroutes/orpages/directories; elsewhere they are judged like any other export.#[cfg_attr(<pred>, test)]and#[cfg_attr(<pred>, tokio::test(…))], without countingtestinside the predicate, as the predicate, or as a path-qualifiedtest::…attribute.kotlin-error-outside-errors-filenow ignores**/src/*Test/**, covering AndroidandroidUnitTestand Kotlin MultiplatformcommonTest,jvmTest,iosTest,jsTest, and so on.initskips writingsgconfig.ymlwith a warning when.pnp.cjsor.pnp.jsis found in the project or an enclosing workspace root, unless the project has its own installed rules directory.Known gap: nested
cfg_attr(a, cfg_attr(b, test))still under-reports Rust tests.Written for commit 495764c. Summary will update on new commits.