fix(cli): tighten please-style check heuristics deferred from #36 (#42)

* fix(cli): tighten filename check heuristics for Dart generators and framework-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

* fix(ast-grep-config): recognize #[cfg_attr(…, test)] as a Rust test attribute

Refs #37

* fix(ast-grep-config): ignore Kotlin Multiplatform test source sets in error-file rule

The kotlin-error-outside-errors-file rule now ignores Kotlin Multiplatform
test source sets.

Refs #37

* fix(cli): skip sgconfig.yml under Yarn Plug'n'Play with a warning

Refs #37

* chore: apply AI code review suggestions

- 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

* chore: apply AI code review suggestions

Document that an installed rules directory overrides an enclosing Plug'n'Play root.

Refs #37

Author: amondnet

packages/ast-grep-config/extract/rust.yml

@@ -187,10 +187,34 @@ rule:
       kind: token_tree
       stopBy: end
 ---
+# A test function attribute: `#[test]`, `#[tokio::test(…)]`, or one applied
+# conditionally, `#[cfg_attr(<predicate>, test)]`. In the conditional form the
+# `test` must be a top-level item after a comma, optionally path-qualified and
+# never followed by `::` (`test::helper` is another attribute), so
+# a `test` inside the predicate (`cfg_attr(all(unix, test), ignore)`) or as the
+# predicate itself (`cfg_attr(test, derive(Debug))`) does not count.
 id: test-attr
 language: rust
 severity: hint
 message: test function attribute
 rule:
   kind: attribute_item
-  regex: ^#\[\s*(\w+::)*test\s*(\(|\])
+  any:
+    - regex: ^#\[\s*(\w+::)*test\s*(\(|\])
+    - has:
+        kind: attribute
+        all:
+          - has:
+              kind: identifier
+              regex: ^cfg_attr$
+          - has:
+              kind: token_tree
+              has:
+                kind: identifier
+                regex: ^test$
+                follows:
+                  regex: ^(,|::)$
+                # `test::helper` names a different attribute.
+                not:
+                  precedes:
+                    regex: ^::$

packages/ast-grep-config/rules/kotlin/kotlin-error-outside-errors-file.md

@@ -67,5 +67,7 @@ class InvoiceService { /* … */ }
 
 ## Exempt files
 
-`Errors.kt` and test source sets (`src/test/`, `src/testFixtures/`,
-`src/androidTest/`) are ignored.
+`Errors.kt` and test source sets are ignored: `src/test/`, `src/testFixtures/`,
+and every source set whose name ends in `Test` (Android `src/androidTest/` and
+`src/androidUnitTest/`, Kotlin Multiplatform `src/commonTest/`, `src/jvmTest/`,
+`src/iosTest/`, `src/jsTest/`, …).

packages/ast-grep-config/rules/kotlin/kotlin-error-outside-errors-file.yml

@@ -38,4 +38,6 @@ ignores:
   - '**/Errors.kt'
   - '**/src/test/**'
   - '**/src/testFixtures/**'
-  - '**/src/androidTest/**'
+  # Android and Kotlin Multiplatform test source sets: androidTest,
+  # androidUnitTest, commonTest, jvmTest, iosTest, jsTest, …
+  - '**/src/*Test/**'

packages/ast-grep-config/test/e2e/sgconfig.test.ts

@@ -70,6 +70,10 @@ const PROJECT = {
   'lib/auth/token_store.dart': 'class InvalidToken implements Exception {}',
   'src/main/kotlin/com/acme/Errors.kt': 'class A : RuntimeException()',
   'src/main/kotlin/com/acme/Billing.kt': 'class B : RuntimeException()',
+  'shared/src/commonMain/kotlin/com/acme/Sync.kt': 'class C : RuntimeException()',
+  'shared/src/commonTest/kotlin/com/acme/Fakes.kt': 'class D : RuntimeException()',
+  'shared/src/iosTest/kotlin/com/acme/Fakes.kt': 'class E : RuntimeException()',
+  'app/src/androidUnitTest/kotlin/com/acme/Fakes.kt': 'class F : RuntimeException()',
   'src/main/java/com/acme/error/AException.java': 'public class AException extends RuntimeException {}',
   'src/androidTest/java/com/acme/CException.java': 'public class CException extends RuntimeException {}',
   'src/main/java/com/acme/BException.java': 'public class BException extends RuntimeException {}',
@@ -84,6 +88,7 @@ const EXPECTED = [
   'lib/auth/token_store.dart:1 dart-error-outside-errors-file',
   'packages/core/plugins/loader.ts:1 ts-no-default-export',
   'pages/about.ts:1 ts-no-default-export',
+  'shared/src/commonMain/kotlin/com/acme/Sync.kt:1 kotlin-error-outside-errors-file',
   'src/card.tsx:1 tsx-no-default-export',
   'src/main/java/com/acme/BException.java:1 java-error-outside-error-package',
   'src/main/kotlin/com/acme/Billing.kt:1 kotlin-error-outside-errors-file',

packages/cli/README.ko.md

@@ -63,6 +63,15 @@ bunx @pleaseai/code-style check --strict   # 발견 사항이 있으면 exit 1 (
 참고하세요. `please-style check`의 발견 사항은 slug를 보여 주며, 각 검사는
 [`please-style check`](#please-style-check)에 설명되어 있습니다.
 
+Yarn Plug'n'Play 프로젝트(프로젝트나 상위 워크스페이스 루트에 `.pnp.cjs` 또는
+`.pnp.js` 파일이 있는 경우)에서는 `init`이 `sgconfig.yml`을 작성하지 않고
+경고만 출력합니다. ast-grep은 디렉터리에서 규칙을 읽는데, Plug'n'Play는
+패키지를 `node_modules` 없이 zip 아카이브로 보관하기 때문입니다. 규칙을
+쓰려면 `.yarnrc.yml`에 `nodeLinker: node-modules`를 설정하고 `yarn install`을
+실행한 뒤 `init`을 다시 실행하세요. 프로젝트 자체에
+`node_modules/@pleaseai/ast-grep-config/rules`가 설치되어 있으면 상위 디렉터리가
+Plug'n'Play여도 `sgconfig.yml`을 작성합니다.
+
 ## `please-style check`
 
 `check`는
@@ -128,7 +137,8 @@ bunx @pleaseai/code-style check --strict   # 발견 사항이 하나라도 있
     `cargo metadata --no-deps --format-version 1`을 읽고 `mod` 선언을 따라가,
     Cargo가 컴파일하지 않는 `tests/**/*.rs` 파일(`tests/common/`과
     `tests/<name>/main.rs` 대상의 하위 모듈은 제외)과 어떤 `mod` 선언도 닿지
-    않는 `#[test]` 파일을 보고합니다. `cargo`가 없으면 알림을 남기고 Rust를
+    않는 `#[test]` 파일(`#[cfg_attr(…, test)]`로 붙인 테스트 속성 포함)을
+    보고합니다. `cargo`가 없으면 알림을 남기고 Rust를
     건너뜁니다.
 - **`test-helpers-in-dedicated-location`**: 지정 위치 밖에서 선언된 헬퍼
   후보(`mock*`, `createMock*`, `fake*`, `stub*`)는 둘 이상의 테스트 파일이

packages/cli/README.md

@@ -63,6 +63,15 @@ with the fix. See the
 structural rules. `please-style check` findings name their slug; each check is
 described under [`please-style check`](#please-style-check).
 
+Under Yarn Plug'n'Play (a `.pnp.cjs` or `.pnp.js` file in the project or an
+enclosing workspace root), `init` does not write `sgconfig.yml` and prints a
+warning instead: ast-grep reads rules from a directory, and Plug'n'Play keeps
+packages in zip archives without `node_modules`. To use the rules, set
+`nodeLinker: node-modules` in `.yarnrc.yml`, run `yarn install`, and run
+`init` again. A project with its own installed
+`node_modules/@pleaseai/ast-grep-config/rules` still gets `sgconfig.yml`, even
+under an enclosing Plug'n'Play directory.
+
 ## `please-style check`
 
 `check` is the layer-3 path checker of
@@ -132,7 +141,8 @@ What it checks:
     `cargo metadata --no-deps --format-version 1` and follows `mod`
     declarations, then reports `tests/**/*.rs` files Cargo never compiles
     (except `tests/common/` and submodules of a `tests/<name>/main.rs`
-    target), and `#[test]` files no `mod` declaration reaches. Without
+    target), and `#[test]` files no `mod` declaration reaches (a test
+    attribute applied with `#[cfg_attr(…, test)]` counts too). Without
     `cargo`, Rust is skipped with a notice.
   - Files come from `git ls-files` (tracked plus untracked-not-ignored) or,
     outside a git work tree, a directory walk (which also skips build and
@@ -176,7 +186,7 @@ their exports cannot be enumerated):
 | TypeScript, TSX | `export default …`, `export { x as default }`, `export { default }` | `default export` |
 | TypeScript, TSX | `export const { a } = obj`, `export const [a] = arr` | `destructured export` |
 | TypeScript, TSX | `export { x as "string name" }` | `string-literal export name` |
-| TypeScript, TSX | a file whose only export is a framework-reserved name: `Route` (TanStack Router), `GET`/`POST`/`PUT`/`PATCH`/`DELETE`/`HEAD`/`OPTIONS`/`ALL` (route handlers), `loader`, `action` | `framework-reserved export` |
+| TypeScript, TSX | a file under a `routes/` or `pages/` directory whose only export is a framework-reserved name: `Route` (TanStack Router), `GET`/`POST`/`PUT`/`PATCH`/`DELETE`/`HEAD`/`OPTIONS`/`ALL` (route handlers), `loader`, `action`. Elsewhere these names are judged like any other export | `framework-reserved export` |
 | Dart | `export 'other.dart';` | `re-export` |
 | Dart | `part 'other.dart';` (generated `*.g.dart`/`*.freezed.dart` parts excluded; any other suffix such as `*.custom.dart` counts) | `library with parts` |
 
@@ -202,8 +212,11 @@ Exempt **by convention** (no finding, no notice):
   `__mocks__/` directory, and anything under the package's test root (`test/`, `tests/`).
 - Dart: `part of` files, files under `integration_test/`, `test_driver/`,
   `bin/`, `tool/`, `example/`, `web/` or the test root, `*_test.dart`,
-  `errors.dart`, generated files whose name has an extra dot (`*.g.dart`,
-  `*.freezed.dart`), and files whose first line is a generated-code header
+  `errors.dart`, generator output named by its suffix (`*.g.dart`,
+  `*.freezed.dart`, `*.mocks.dart`, `*.gr.dart`, `*.config.dart`,
+  `*.gen.dart`, `*.chopper.dart`, `*.mapper.dart`, `*.gql.dart`,
+  `*.graphql.dart`, `*.pb.dart`, `*.pbenum.dart`, `*.pbjson.dart`,
+  `*.pbserver.dart`, `*.pbgrpc.dart`), and files whose first line is a generated-code header
   (`// GENERATED CODE` or `// File generated by`, as in `firebase_options.dart`).
 
 Names are compared by a separator-free key: the name is split into words at

packages/cli/src/appliers.ts

@@ -1,7 +1,7 @@
 import type { Tool, ToolApplyResult, ToolContext } from './tools.js'
 import { copyFileSync, existsSync, writeFileSync } from 'node:fs'
-import { resolve as joinPath } from 'node:path'
-import { confirm, isCancel } from '@clack/prompts'
+import { dirname, resolve as joinPath } from 'node:path'
+import { confirm, isCancel, log } from '@clack/prompts'
 import {
   isPackageInstalled,
   readFileOrNull,
@@ -228,10 +228,34 @@ function listsAstGrepRules(yaml: string): boolean {
   return entries.some(e => normalizeRuleDir(e) === AST_GREP_RULES)
 }
 
+/**
+ * Yarn Plug'n'Play installs packages as zip archives without `node_modules`, so the rules path above does not exist.
+ * A workspace package's `.pnp.cjs` lives at the workspace root, so the enclosing directories are searched too;
+ * an installed rules directory means this project is not under Plug'n'Play, whatever an ancestor holds.
+ */
+function usesPlugNPlay(cwd: string): boolean {
+  if (existsSync(joinPath(cwd, AST_GREP_RULES))) {
+    return false
+  }
+  for (let dir = joinPath(cwd); ; dir = dirname(dir)) {
+    if (existsSync(joinPath(dir, '.pnp.cjs')) || existsSync(joinPath(dir, '.pnp.js'))) {
+      return true
+    }
+    if (dirname(dir) === dir) {
+      return false
+    }
+  }
+}
+
 export async function applyAstGrep(ctx: ToolContext): Promise<ToolApplyResult> {
   const result: ToolApplyResult = { created: [], updated: [], skipped: [] }
   const target = joinPath(ctx.cwd, 'sgconfig.yml')
   const label = 'sgconfig.yml'
+  if (usesPlugNPlay(ctx.cwd)) {
+    log.warn(t('astGrepPlugNPlay'))
+    result.skipped.push(label)
+    return result
+  }
   const existing = readFileOrNull(target)
 
   if (existing == null) {

packages/cli/src/check/check-filenames.ts

@@ -39,8 +39,8 @@ export interface FilenameLanguage {
   nameRules: string[]
   /** Rule ids whose presence means the file's exports cannot be enumerated; such files are skipped. */
   opaqueRules?: string[]
-  /** Export names a framework dictates (a file whose only export is one is withheld, not judged). */
-  reservedNames?: ReadonlySet<string>
+  /** Whether a framework dictates the export `name` in `file` (a file whose only export is one is withheld, not judged). */
+  isReserved?: (name: string, file: string) => boolean
   normalize: (name: string) => string
   isPublic: (name: string) => boolean
   /** File name without extension, or `null` when the file is exempt. */
@@ -63,6 +63,14 @@ function underTestRoot(file: string, unit: Unit | undefined): boolean {
 /** Names a router or runtime framework requires a file to export (TanStack Router `Route`, HTTP verb handlers, Remix `loader`/`action`). */
 const FRAMEWORK_RESERVED = new Set(['Route', 'GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS', 'ALL', 'loader', 'action'])
 
+/** Directories a file-based router maps to URLs: `routes/` (TanStack Router, Remix, SolidStart) and `pages/` (Next.js API routes, Astro endpoints). */
+function inRouteDir(file: string): boolean {
+  return file.split('/').slice(0, -1).some(seg => seg === 'routes' || seg === 'pages')
+}
+
+/** Last stem segment of a Dart code generator's output (`*.g.dart`, `*.freezed.dart`, Mockito `*.mocks.dart`, auto_route `*.gr.dart`, protobuf `*.pb*.dart`, …). */
+const DART_GENERATOR_SUFFIX = /\.(?:g|freezed|mocks|gr|config|gen|chopper|mapper|gql|graphql|pb|pbenum|pbjson|pbserver|pbgrpc)$/
+
 const GENERATED_HEADER = /^\/\/\s*(?:GENERATED CODE|File generated by)/
 
 /** Dart files whose first line is a generated-code header; their name is dictated by the generator. */
@@ -97,7 +105,7 @@ export function typescriptFilenames(units: Unit[]): FilenameLanguage {
     language: 'typescript',
     nameRules: ['export-name'],
     opaqueRules: ['export-destructure', 'export-opaque'],
-    reservedNames: FRAMEWORK_RESERVED,
+    isReserved: (name, file) => FRAMEWORK_RESERVED.has(name) && inRouteDir(file),
     normalize: toKebabCase,
     isPublic: () => true,
     units,
@@ -137,8 +145,8 @@ export function dartFilenames(units: Unit[], partFiles: Set<string>): FilenameLa
         return null
       }
       const stem = name.slice(0, -'.dart'.length)
-      // `_test.dart`, the error file, and generated `*.g.dart`/`*.freezed.dart`.
-      if (stem.endsWith('_test') || stem === 'errors' || stem.includes('.')) {
+      // `_test.dart`, the error file, and generator output named by the generator.
+      if (stem.endsWith('_test') || stem === 'errors' || DART_GENERATOR_SUFFIX.test(stem)) {
         return null
       }
       return stem
@@ -184,7 +192,7 @@ export function inspectFilenames(matches: ExtractMatch[], lang: FilenameLanguage
       opaque.set(m.file, (opaque.get(m.file) ?? new Set()).add('re-export'))
       continue
     }
-    if (lang.reservedNames?.has(m.text) === true) {
+    if (lang.isReserved?.(m.text, m.file) === true) {
       opaque.set(m.file, (opaque.get(m.file) ?? new Set()).add('framework-reserved export'))
     }
     const names = byFile.get(m.file) ?? new Map<string, number>()

packages/cli/src/i18n.ts

@@ -17,6 +17,7 @@ interface Messages {
   created: (file: string) => string
   updated: (file: string) => string
   agentsBlockUpdated: string
+  astGrepPlugNPlay: string
   doctorHeading: string
   doctorOk: string
   doctorMissing: string
@@ -45,6 +46,7 @@ const messages: Record<Locale, Messages> = {
     created: file => `Created ${file}`,
     updated: file => `Updated ${file}`,
     agentsBlockUpdated: 'Refreshed AGENTS.md pleaseai-code-style block.',
+    astGrepPlugNPlay: 'Yarn Plug\'n\'Play has no node_modules, so ast-grep cannot read the rules from node_modules/@pleaseai/ast-grep-config. Set `nodeLinker: node-modules` in .yarnrc.yml, run yarn install, and run init again.',
     doctorHeading: 'Project status',
     doctorOk: 'OK',
     doctorMissing: 'missing',
@@ -99,6 +101,7 @@ Exit codes: 0 ok or warnings only, 1 findings with --strict, 2 usage or config e
     created: file => `${file} 생성됨`,
     updated: file => `${file} 업데이트됨`,
     agentsBlockUpdated: 'AGENTS.md의 pleaseai-code-style 블록을 갱신했습니다.',
+    astGrepPlugNPlay: 'Yarn Plug\'n\'Play는 node_modules를 만들지 않아 ast-grep이 node_modules/@pleaseai/ast-grep-config의 규칙을 읽을 수 없습니다. .yarnrc.yml에 `nodeLinker: node-modules`를 설정하고 yarn install 후 init을 다시 실행하세요.',
     doctorHeading: '프로젝트 상태',
     doctorOk: '정상',
     doctorMissing: '없음',

packages/cli/test/appliers.test.ts

@@ -1,4 +1,4 @@
-import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
@@ -61,6 +61,30 @@ describe('ast-grep tool', () => {
     expect(readFileSync(join(dir, 'sgconfig.yml'), 'utf-8')).toContain('ruleDirs:\n  - node_modules/@pleaseai/ast-grep-config/rules\n')
   })
 
+  test('does not write sgconfig.yml under Yarn Plug\'n\'Play, where node_modules does not exist', async () => {
+    writeFileSync(join(dir, '.pnp.cjs'), '')
+    const result = await applyAstGrep({ cwd: dir, autoAccept: true })
+    expect(result).toEqual({ created: [], updated: [], skipped: ['sgconfig.yml'] })
+    expect(existsSync(join(dir, 'sgconfig.yml'))).toBe(false)
+  })
+
+  test('detects Plug\'n\'Play from a workspace package whose .pnp.cjs is at the workspace root', async () => {
+    writeFileSync(join(dir, '.pnp.cjs'), '')
+    const pkg = join(dir, 'packages', 'app')
+    mkdirSync(pkg, { recursive: true })
+    const result = await applyAstGrep({ cwd: pkg, autoAccept: true })
+    expect(result.skipped).toEqual(['sgconfig.yml'])
+    expect(existsSync(join(pkg, 'sgconfig.yml'))).toBe(false)
+  })
+
+  test('writes sgconfig.yml for a node_modules project nested under a Plug\'n\'Play directory', async () => {
+    writeFileSync(join(dir, '.pnp.cjs'), '')
+    const project = join(dir, 'examples', 'app')
+    mkdirSync(join(project, 'node_modules', '@pleaseai', 'ast-grep-config', 'rules'), { recursive: true })
+    const result = await applyAstGrep({ cwd: project, autoAccept: true })
+    expect(result.created).toEqual(['sgconfig.yml'])
+  })
+
   test('overwrites a different sgconfig.yml when overwrites are accepted', async () => {
     writeFileSync(join(dir, 'sgconfig.yml'), 'ruleDirs:\n  - rules\n')
     const result = await applyAstGrep({ cwd: dir, autoAccept: true })