Skip to content

Commit 7d2a45c

Browse files
Copilotafranken
andcommitted
Extract Kotlin guidelines to docs/KOTLIN.md and add links across docs, skills, and agent files
Co-authored-by: afranken <763000+afranken@users.noreply.github.com>
1 parent 2e37df3 commit 7d2a45c

File tree

7 files changed

+83
-37
lines changed

7 files changed

+83
-37
lines changed

.claude/skills/document/SKILL.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Read `AGENTS.md` (root + relevant module) before making changes.
1414
| `README.md` | End users | Usage, configuration, operations table |
1515
| `CHANGELOG.md` | End users | Version history, breaking changes |
1616
| `docs/TESTING.md` | Contributors / AI agents | Testing strategy, base classes, patterns, commands |
17+
| `docs/KOTLIN.md` | Contributors / AI agents | Kotlin idioms, naming conventions, anti-patterns, KDoc |
1718
| `AGENTS.md` (root + modules) | AI agents | Architecture, conventions, guardrails |
1819
| `.github/CONTRIBUTING.md` | Contributors | Dev setup, code reviews |
1920

.claude/skills/refactor/SKILL.md

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -21,40 +21,15 @@ Remove comments that restate code. Add comments for *why* a decision was made, e
2121

2222
### Meaningful Naming Over Comments
2323

24-
If you need a comment to explain *what* code does, rename instead:
25-
- Booleans: `is-`/`has-`/`should-`/`can-` prefixes
26-
- Collections: plural nouns
27-
- Functions: verb phrases (`verifyBucketExists`, `resolveVersionId`)
28-
- Avoid abbreviations (`bucketMetadata` not `bktMd`)
24+
If you need a comment to explain *what* code does, rename instead. See **[docs/KOTLIN.md](../../../docs/KOTLIN.md)** for naming conventions.
2925

30-
### Idiomatic Kotlin
26+
### Idiomatic Kotlin & Common Anti-Patterns
3127

32-
- **`.let`/`.also`**: Use when they improve readability, not gratuitously
33-
- **Expression bodies**: For single-expression functions
34-
- **Null safety**: `?.`, `?:` over `if (x != null)` checks
35-
- **Named `it`**: Always name in nested or non-trivial lambdas
36-
- **`when`**: Over `if-else` chains with 3+ branches
37-
- **Early returns**: Flatten deeply nested code
38-
- **Extract functions**: Break up methods longer than ~30 lines
39-
40-
### Common Anti-Patterns to Fix
41-
42-
| Anti-Pattern | Refactor To |
43-
|---|---|
44-
| `if (x != null) { x.foo() }` | `x?.foo()` |
45-
| `if (x == null) throw ...` | `x ?: throw ...` or `requireNotNull(x)` |
46-
| `list.size == 0` / `list.size > 0` | `list.isEmpty()` / `list.isNotEmpty()` |
47-
| `"" + value` | `"$value"` |
48-
| `Collections.emptyList()` | `emptyList()` |
49-
| `object.equals(other)` | `object == other` |
50-
| `!(x is Foo)` / `!(list.contains(x))` | `x !is Foo` / `x !in list` |
51-
| `for + add` loops | `.map { ... }` |
52-
| Empty catch blocks | At minimum, log the exception |
53-
| Magic numbers/strings | Named constants |
28+
See **[docs/KOTLIN.md](../../../docs/KOTLIN.md)** for the full list of Kotlin idioms, common anti-patterns and their fixes, and scope function guidance.
5429

5530
### KDoc for Public APIs
5631

57-
Document what, why, and gotchas. Link to AWS API docs where relevant.
32+
Document what, why, and gotchas. Link to AWS API docs where relevant. See **[docs/KOTLIN.md](../../../docs/KOTLIN.md)** for KDoc conventions.
5833

5934
## Checklist
6035

.claude/skills/review/SKILL.md

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,8 @@ Evaluate changes against these categories, in priority order:
3535
- Backtick names, `internal class`, AssertJ assertions?
3636

3737
### 4. Kotlin Idioms
38-
- Null safety (`?.`, `?:`) over explicit null checks
39-
- Expression bodies for single-expression functions
40-
- Named `it` in nested/non-trivial lambdas
41-
- `when` over long if-else chains
42-
- `isEmpty()`/`isNotEmpty()` over size checks
43-
- String templates over concatenation
38+
39+
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).
4440

4541
### 5. Documentation & Changelog
4642
- Is `CHANGELOG.md` updated for user-facing changes?

.claude/skills/test/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ description: Write, update, or fix tests. Use when asked to test code, create te
55

66
# Test Skill — S3Mock
77

8-
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.
8+
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.
99

1010
## Key Conventions (from AGENTS.md)
1111

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ docker/ # Docker image build
6464

6565
## Code Style
6666

67-
**Kotlin idioms**: Data classes for DTOs, null safety, expression bodies, constructor injection
67+
See **[docs/KOTLIN.md](docs/KOTLIN.md)** for Kotlin idioms, naming conventions, common anti-patterns, and KDoc guidelines.
6868

6969
**Spring**: `@RestController`, `@Service`, `@Component`, constructor injection over field injection
7070

docs/KOTLIN.md

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# Kotlin Guidelines — S3Mock
2+
3+
Canonical reference for Kotlin idioms, naming conventions, and code quality standards used across this project.
4+
5+
## Idioms
6+
7+
### Null Safety
8+
- Use `?.`, `?:`, and nullable types instead of explicit null checks
9+
- `x?.foo()` over `if (x != null) { x.foo() }`
10+
- `x ?: throw ...` or `requireNotNull(x)` over `if (x == null) throw ...`
11+
12+
### Immutability
13+
- Prefer `val` over `var`, especially for public API properties
14+
15+
### Expression Bodies
16+
- Use for single-expression functions: `fun foo() = bar()`
17+
18+
### Lambda Parameters
19+
- Always name `it` in nested or non-trivial lambdas to avoid shadowing
20+
- `.map { part -> ... }` instead of `.map { it.name }`
21+
22+
### `when` Expressions
23+
- Prefer `when` over `if-else` chains with 3+ branches
24+
25+
### Scope Functions
26+
- Use `.let`/`.also` when they improve readability, not gratuitously
27+
- Use early returns to flatten deeply nested code
28+
- Extract functions: break up methods longer than ~30 lines
29+
30+
### Collections
31+
- `list.isEmpty()` / `list.isNotEmpty()` over `list.size == 0` / `list.size > 0`
32+
33+
### String Templates
34+
- Use `"$value"` over `"" + value` concatenation
35+
36+
### Kotlin Stdlib
37+
- Prefer Kotlin stdlib / JDK APIs over adding new third-party libraries (no Apache Commons)
38+
39+
## Common Anti-Patterns
40+
41+
| Anti-Pattern | Refactor To |
42+
|---|---|
43+
| `if (x != null) { x.foo() }` | `x?.foo()` |
44+
| `if (x == null) throw ...` | `x ?: throw ...` or `requireNotNull(x)` |
45+
| `list.size == 0` / `list.size > 0` | `list.isEmpty()` / `list.isNotEmpty()` |
46+
| `"" + value` | `"$value"` |
47+
| `Collections.emptyList()` | `emptyList()` |
48+
| `object.equals(other)` | `object == other` |
49+
| `!(x is Foo)` / `!(list.contains(x))` | `x !is Foo` / `x !in list` |
50+
| `for + add` loops | `.map { ... }` |
51+
| Empty catch blocks | At minimum, log the exception |
52+
| Magic numbers/strings | Named constants |
53+
54+
## Naming Conventions
55+
56+
- **Booleans**: `is-`/`has-`/`should-`/`can-` prefixes
57+
- **Collections**: plural nouns
58+
- **Functions**: verb phrases (`verifyBucketExists`, `resolveVersionId`)
59+
- **Avoid abbreviations**: `bucketMetadata` not `bktMd`
60+
61+
## Test Naming
62+
63+
- **Backtick names**: Use descriptive sentences — `` fun `should create bucket successfully`() ``
64+
- **Legacy names**: Refactor `testSomething` camelCase names to backtick style when touching existing tests
65+
- **Visibility**: Mark test classes as `internal`
66+
67+
## KDoc
68+
69+
- Use `/** */` for public APIs; `//` inline comments for rationale
70+
- Comments explain **why**, never **what** — remove comments that restate the code
71+
- Add comments for edge cases, non-obvious S3 semantics, or workarounds
72+
- Link to AWS API docs or GitHub issues where relevant

docs/TESTING.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ Access `serviceEndpoint`, `serviceEndpointHttp`, and `serviceEndpointHttps` from
6565

6666
## Conventions
6767

68+
See **[docs/KOTLIN.md](KOTLIN.md)** for Kotlin naming conventions (backtick test names, `internal` visibility, naming patterns).
69+
6870
- **Naming**: Backtick names with descriptive sentences — `` fun `should create bucket successfully`() ``
6971
- **Visibility**: Mark test classes `internal`
7072
- **Pattern**: Arrange-Act-Assert

0 commit comments

Comments
 (0)