Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ docker/ # Docker image build

See **[docs/KOTLIN.md](docs/KOTLIN.md)** for Kotlin idioms, naming conventions, common anti-patterns, and KDoc guidelines.

See **[docs/JAVA.md](docs/JAVA.md)** for Java idioms, naming conventions, common anti-patterns, and Javadoc guidelines.

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

## XML Serialization
Expand Down
74 changes: 74 additions & 0 deletions docs/JAVA.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Java Guidelines — S3Mock

Canonical reference for Java idioms, naming conventions, and code quality standards used across this project.

## Style

Java code follows the **[Google Java Style Guide](https://google.github.io/styleguide/javaguide.html)** enforced by Checkstyle (`etc/checkstyle.xml`).

Key rules:
- **Indentation**: 2 spaces (no tabs)
- **Line length**: 120 characters maximum
- **Braces**: Always use braces for `if`, `for`, `while`, `do` blocks
- **Imports**: Static imports first, then third-party packages; alphabetical within groups; no wildcard imports
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import-order guidance here doesn’t match the enforced Checkstyle config: CustomImportOrder is configured as STATIC###THIRD_PARTY_PACKAGE, which effectively means “static imports first, then all non-static imports” (no separate standard/third-party grouping). Consider rewording this bullet to reflect the actual rule to avoid confusion.

Suggested change
- **Imports**: Static imports first, then third-party packages; alphabetical within groups; no wildcard imports
- **Imports**: Static imports first, then all non-static imports; alphabetical within each group; no wildcard imports

Copilot uses AI. Check for mistakes.

## Modern Java Idioms

### Local Type Inference
- Use `var` for local variables when the type is clear from context:
```java
var uploadFile = new File(UPLOAD_FILE_NAME);
var response = s3Client.getObject(...);
```
- Avoid `var` when the inferred type would be ambiguous or unclear

### Collections
- `list.size() == 0` / `list.size() > 0` → `list.isEmpty()` / `!list.isEmpty()`
- Use `List.of(...)`, `Map.of(...)` for immutable collections instead of `Collections.unmodifiableList(...)`
- Prefer streams over explicit loops for transformations:
```java
buckets.stream().map(Bucket::name).collect(Collectors.toSet())
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Java snippet is missing a trailing semicolon, so it isn’t a copy/pasteable example. Consider updating it to a syntactically complete statement (and optionally include/import Collectors if you want it to compile standalone).

Suggested change
buckets.stream().map(Bucket::name).collect(Collectors.toSet())
buckets.stream().map(Bucket::name).collect(Collectors.toSet());

Copilot uses AI. Check for mistakes.
```

### Switch Expressions
- Prefer switch expressions over `if-else` chains with 3+ branches

### Text Blocks
- Use text blocks for multi-line strings

## Common Anti-Patterns

| Anti-Pattern | Refactor To |
|---|---|
| `list.size() == 0` | `list.isEmpty()` |
| `Collections.emptyList()` | `List.of()` |
| `Collections.unmodifiableList(new ArrayList<>(...))` | `List.copyOf(...)` |
| `"" + value` | `String.valueOf(value)` or `String.format(...)` |
| Empty catch blocks | At minimum, log the exception |
| Magic numbers/strings | Named constants |
Comment on lines +41 to +48
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The anti-patterns table is not valid Markdown because the rows start with || instead of |. This will render incorrectly on GitHub; use a single leading pipe for each row (header, separator, and body).

Copilot uses AI. Check for mistakes.

## Naming Conventions

Follows [Google Java Style](https://google.github.io/styleguide/javaguide.html#s5-naming):
- **Classes/Interfaces/Enums**: `UpperCamelCase`
- **Methods/Variables**: `lowerCamelCase`
- **Constants** (`static final`): `UPPER_SNAKE_CASE`
- **Booleans**: `is-`/`has-`/`should-`/`can-` prefixes
- **Collections**: plural nouns
- **Avoid abbreviations**: `bucketMetadata` not `bktMd`
- **Abbreviations** in type names: at most 4 consecutive uppercase letters (e.g., `KmsKeyRef` not `KMSKeyRef`)

## Test Naming

- **Method names**: Use descriptive verb phrases — `shouldUploadAndDownloadObject`, `defaultBucketsGotCreated`
- **Avoid**: generic names like `testSomething` or `test1`
- **Pattern**: Arrange-Act-Assert

## Javadoc

- Use `/** */` for public APIs; `//` inline comments for rationale
- Comments explain **why**, never **what** — remove comments that restate the code
- Javadoc tag order: `@param`, `@return`, `@throws`, `@deprecated`
- Add comments for edge cases, non-obvious S3 semantics, or workarounds
- Link to AWS API docs or GitHub issues where relevant
- Single-line Javadoc is allowed: `/** Short description. */`
Loading