diff --git a/.claude/skills/document/SKILL.md b/.claude/skills/document/SKILL.md index c530e62b5..f27e27c62 100644 --- a/.claude/skills/document/SKILL.md +++ b/.claude/skills/document/SKILL.md @@ -14,6 +14,7 @@ Read `AGENTS.md` (root + relevant module) before making changes. | `README.md` | End users | Usage, configuration, operations table | | `CHANGELOG.md` | End users | Version history, breaking changes | | `docs/TESTING.md` | Contributors / AI agents | Testing strategy, base classes, patterns, commands | +| `docs/KOTLIN.md` | Contributors / AI agents | Kotlin idioms, naming conventions, anti-patterns, KDoc | | `AGENTS.md` (root + modules) | AI agents | Architecture, conventions, guardrails | | `.github/CONTRIBUTING.md` | Contributors | Dev setup, code reviews | diff --git a/.claude/skills/refactor/SKILL.md b/.claude/skills/refactor/SKILL.md index 1073e9791..079193155 100644 --- a/.claude/skills/refactor/SKILL.md +++ b/.claude/skills/refactor/SKILL.md @@ -21,40 +21,15 @@ Remove comments that restate code. Add comments for *why* a decision was made, e ### Meaningful Naming Over Comments -If you need a comment to explain *what* code does, rename instead: -- Booleans: `is-`/`has-`/`should-`/`can-` prefixes -- Collections: plural nouns -- Functions: verb phrases (`verifyBucketExists`, `resolveVersionId`) -- Avoid abbreviations (`bucketMetadata` not `bktMd`) +If you need a comment to explain *what* code does, rename instead. See **[docs/KOTLIN.md](../../../docs/KOTLIN.md)** for naming conventions. -### Idiomatic Kotlin +### Idiomatic Kotlin & Common Anti-Patterns -- **`.let`/`.also`**: Use when they improve readability, not gratuitously -- **Expression bodies**: For single-expression functions -- **Null safety**: `?.`, `?:` over `if (x != null)` checks -- **Named `it`**: Always name in nested or non-trivial lambdas -- **`when`**: Over `if-else` chains with 3+ branches -- **Early returns**: Flatten deeply nested code -- **Extract functions**: Break up methods longer than ~30 lines - -### Common Anti-Patterns to Fix - -| Anti-Pattern | Refactor To | -|---|---| -| `if (x != null) { x.foo() }` | `x?.foo()` | -| `if (x == null) throw ...` | `x ?: throw ...` or `requireNotNull(x)` | -| `list.size == 0` / `list.size > 0` | `list.isEmpty()` / `list.isNotEmpty()` | -| `"" + value` | `"$value"` | -| `Collections.emptyList()` | `emptyList()` | -| `object.equals(other)` | `object == other` | -| `!(x is Foo)` / `!(list.contains(x))` | `x !is Foo` / `x !in list` | -| `for + add` loops | `.map { ... }` | -| Empty catch blocks | At minimum, log the exception | -| Magic numbers/strings | Named constants | +See **[docs/KOTLIN.md](../../../docs/KOTLIN.md)** for the full list of Kotlin idioms, common anti-patterns and their fixes, and scope function guidance. ### KDoc for Public APIs -Document what, why, and gotchas. Link to AWS API docs where relevant. +Document what, why, and gotchas. Link to AWS API docs where relevant. See **[docs/KOTLIN.md](../../../docs/KOTLIN.md)** for KDoc conventions. ## Checklist diff --git a/.claude/skills/review/SKILL.md b/.claude/skills/review/SKILL.md index 4a6c12ef8..e097c41f0 100644 --- a/.claude/skills/review/SKILL.md +++ b/.claude/skills/review/SKILL.md @@ -35,12 +35,8 @@ Evaluate changes against these categories, in priority order: - Backtick names, `internal class`, AssertJ assertions? ### 4. Kotlin Idioms -- Null safety (`?.`, `?:`) over explicit null checks -- Expression bodies for single-expression functions -- Named `it` in nested/non-trivial lambdas -- `when` over long if-else chains -- `isEmpty()`/`isNotEmpty()` over size checks -- String templates over concatenation + +See **[docs/KOTLIN.md](../../../docs/KOTLIN.md)** for the full list of idioms and anti-patterns to check (null safety, expression bodies, named `it`, `when`, `isEmpty()`/`isNotEmpty()`, string templates). ### 5. Documentation & Changelog - Is `CHANGELOG.md` updated for user-facing changes? diff --git a/.claude/skills/test/SKILL.md b/.claude/skills/test/SKILL.md index 31ce67168..91ee93c57 100644 --- a/.claude/skills/test/SKILL.md +++ b/.claude/skills/test/SKILL.md @@ -5,7 +5,7 @@ description: Write, update, or fix tests. Use when asked to test code, create te # Test Skill — S3Mock -Read **[docs/TESTING.md](../../../docs/TESTING.md)** and `AGENTS.md` (root + relevant module) before writing tests — they define test types, base classes, naming conventions, and running commands. +Read **[docs/TESTING.md](../../../docs/TESTING.md)**, **[docs/KOTLIN.md](../../../docs/KOTLIN.md)**, and `AGENTS.md` (root + relevant module) before writing tests — they define test types, base classes, naming conventions, and running commands. ## Key Conventions (from AGENTS.md) diff --git a/AGENTS.md b/AGENTS.md index 006118701..70ade24a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,7 +64,7 @@ docker/ # Docker image build ## Code Style -**Kotlin idioms**: Data classes for DTOs, null safety, expression bodies, constructor injection +See **[docs/KOTLIN.md](docs/KOTLIN.md)** for Kotlin idioms, naming conventions, common anti-patterns, and KDoc guidelines. **Spring**: `@RestController`, `@Service`, `@Component`, constructor injection over field injection diff --git a/docs/KOTLIN.md b/docs/KOTLIN.md new file mode 100644 index 000000000..d7244d6a2 --- /dev/null +++ b/docs/KOTLIN.md @@ -0,0 +1,72 @@ +# Kotlin Guidelines — S3Mock + +Canonical reference for Kotlin idioms, naming conventions, and code quality standards used across this project. + +## Idioms + +### Null Safety +- Use `?.`, `?:`, and nullable types instead of explicit null checks +- `x?.foo()` over `if (x != null) { x.foo() }` +- `x ?: throw ...` or `requireNotNull(x)` over `if (x == null) throw ...` + +### Immutability +- Prefer `val` over `var`, especially for public API properties + +### Expression Bodies +- Use for single-expression functions: `fun foo() = bar()` + +### Lambda Parameters +- Always name `it` in nested or non-trivial lambdas to avoid shadowing +- `.map { part -> ... }` instead of `.map { it.name }` + +### `when` Expressions +- Prefer `when` over `if-else` chains with 3+ branches + +### Scope Functions +- Use `.let`/`.also` when they improve readability, not gratuitously +- Use early returns to flatten deeply nested code +- Extract functions: break up methods longer than ~30 lines + +### Collections +- `list.isEmpty()` / `list.isNotEmpty()` over `list.size == 0` / `list.size > 0` + +### String Templates +- Use `"$value"` over `"" + value` concatenation + +### Kotlin Stdlib +- Prefer Kotlin stdlib / JDK APIs over adding new third-party libraries (no Apache Commons) + +## Common Anti-Patterns + +| Anti-Pattern | Refactor To | +|---|---| +| `if (x != null) { x.foo() }` | `x?.foo()` | +| `if (x == null) throw ...` | `x ?: throw ...` or `requireNotNull(x)` | +| `list.size == 0` / `list.size > 0` | `list.isEmpty()` / `list.isNotEmpty()` | +| `"" + value` | `"$value"` | +| `Collections.emptyList()` | `emptyList()` | +| `object.equals(other)` | `object == other` | +| `!(x is Foo)` / `!(list.contains(x))` | `x !is Foo` / `x !in list` | +| `for + add` loops | `.map { ... }` | +| Empty catch blocks | At minimum, log the exception | +| Magic numbers/strings | Named constants | + +## Naming Conventions + +- **Booleans**: `is-`/`has-`/`should-`/`can-` prefixes +- **Collections**: plural nouns +- **Functions**: verb phrases (`verifyBucketExists`, `resolveVersionId`) +- **Avoid abbreviations**: `bucketMetadata` not `bktMd` + +## Test Naming + +- **Backtick names**: Use descriptive sentences — `` fun `should create bucket successfully`() `` +- **Legacy names**: Refactor `testSomething` camelCase names to backtick style when touching existing tests +- **Visibility**: Mark test classes as `internal` + +## KDoc + +- Use `/** */` for public APIs; `//` inline comments for rationale +- Comments explain **why**, never **what** — remove comments that restate the code +- Add comments for edge cases, non-obvious S3 semantics, or workarounds +- Link to AWS API docs or GitHub issues where relevant diff --git a/docs/TESTING.md b/docs/TESTING.md index 0f01c1c49..615ad3fe2 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -65,6 +65,8 @@ Access `serviceEndpoint`, `serviceEndpointHttp`, and `serviceEndpointHttps` from ## Conventions +See **[docs/KOTLIN.md](KOTLIN.md)** for Kotlin naming conventions (backtick test names, `internal` visibility, naming patterns). + - **Naming**: Backtick names with descriptive sentences — `` fun `should create bucket successfully`() `` - **Visibility**: Mark test classes `internal` - **Pattern**: Arrange-Act-Assert