Skip to content

Commit 3067b3e

Browse files
committed
Code review agent: more classLoaderMatcher guidance
1 parent 3263e35 commit 3067b3e

1 file changed

Lines changed: 18 additions & 14 deletions

File tree

.github/agents/knowledge/javaagent-module-patterns.md

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -103,22 +103,24 @@ One positive landmark class can sometimes provide **both** bounds: its presence
103103
module is at least version `X.Y`, and the same class disappears in `Y.Z`, so it also excludes
104104
`Y.Z+`. In that case, use a combined comment such as `// added in X.Y, removed in Y.Z`.
105105

106-
For a single-item `hasClassesNamed(...)` check, prefer the compact form with the version
107-
comment above the statement or expression that uses the matcher, instead of splitting the
108-
single string onto its own line:
106+
When the entire `classLoaderMatcher()` return is a single `hasClassesNamed(...)` call with
107+
one class — no `.and(...)` chaining — prefer the compact form with the version comment above
108+
the statement or expression, instead of splitting the single string onto its own line:
109109

110110
```java
111111
// added in 3.0
112112
return hasClassesNamed("jakarta.faces.context.FacesContext");
113113
```
114114

115-
The same compact style applies to single negated exclusions:
116-
117115
```java
118116
// added in 8.10 (native OTel support)
119-
.and(not(hasClassesNamed("co.elastic.clients.transport.instrumentation.Instrumentation")));
117+
return not(hasClassesNamed("co.elastic.clients.transport.instrumentation.Instrumentation"));
120118
```
121119

120+
The compact form does **not** apply when matchers are chained (e.g., positive + negated).
121+
In that case, follow the multi-class rule: place each version comment directly above its
122+
class name string.
123+
122124
**How to identify ceiling classes**: look for a **newer sibling module** for the same
123125
library, such as `mongo-4.0` next to `mongo-3.7`. If the newer module's
124126
`classLoaderMatcher()` checks a different variant of the same class, such as
@@ -211,9 +213,10 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
211213
the module truly depends on an added or removed landmark class that muzzle does not inspect.
212214
- **Version comments are required on landmark classes.** For multi-class checks, or whenever
213215
the landmark version differs from the module's base version, every `hasClassesNamed()` call
214-
needs a role comment. For a single-item `hasClassesNamed(...)` check, including a negated
215-
exclusion in `.and(not(...))`, prefer the compact form with the comment above the statement
216-
or expression instead of placing the only string on its own line.
216+
needs a role comment. When the entire return expression is a single `hasClassesNamed(...)`
217+
call with one class (no `.and(...)` chaining), prefer the compact form with the version
218+
comment above the statement or expression. When matchers are chained, place each version
219+
comment directly above its class name string.
217220
- Positive floor class → `// added in X.Y`, optionally with brief, non-duplicative context
218221
such as `renamed from javax` or `backported to 2.12.3`.
219222
- Positive ceiling class → `// removed in Y.Z`, optionally with brief, non-duplicative
@@ -226,11 +229,12 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
226229
when that class disappeared, not when it first appeared. Conversely, do use `// added in`
227230
for a **negated** exclusion class, because its first appearance is what starts the excluded
228231
range.
229-
- **Single-class landmark comments are required.** When a single-class `hasClassesNamed(...)`
230-
check establishes the module's lower bound, or when a single-class negated check establishes
231-
an excluded upper range, use the compact form with the version comment immediately above the
232-
statement or expression that uses the matcher. Parenthetical context is fine only when it
233-
adds interesting, non-duplicative signal. If the same positive class also establishes the
232+
- **Single-class landmark comments are required.** When the entire return expression is a
233+
single `hasClassesNamed(...)` call with one class (no chaining), use the compact form with
234+
the version comment above the statement or expression. When matchers are chained (e.g.,
235+
positive `.and(not(...))` or multi-argument), place each version comment directly above its
236+
class name string — not above the outer `.and(` expression. Parenthetical context is fine
237+
only when it adds interesting, non-duplicative signal. If the same positive class also establishes the
234238
upper bound because it is removed or replaced in a newer sibling version, include that too,
235239
for example `// added in X.Y, removed in Y.Z`. First validate the stated boundaries from
236240
repository or upstream evidence; do not infer them from the module name alone.

0 commit comments

Comments
 (0)