Skip to content

Commit a4c3746

Browse files
LTSCommerceclaude
andcommitted
feat(api-surface): auto-skip autoload-dev + @api-must-not-leak-@internal rule
Two extensions to the package-type-aware API-surface enforcement, both default-on and library-only: - RequireApiOrInternalTagRule now auto-skips class-likes under an autoload-dev PSR-4 namespace (tests, dev tooling, QA config) — read from composer.json via the new DevAutoloadNamespaceReader. Dev-only code is never shipped runtime surface, so a library no longer hand-tags its own test suite. The explicit ignoredNamespacePrefixes parameter remains for runtime generated/managed trees. - New ApiMustNotExposeInternalRule (phpqaci.apiMustNotExposeInternal): an @api class-like must not expose an @internal one (same package/root namespace) through its public surface — public-method param/return types or public properties. Catches the hollow-@api leak that the presence rule alone misses; a consumer using the @api type would be forced to touch the @internal one. Also: rename ApiOrInternalTagVerdict -> ApiOrInternalTagVerdictEnum to satisfy the shipped PHPArkitect *Enum suffix tier; whitelist PHPStan\Node\InClassNode + PHPStan\Type\Type for composer-require-checker; docs updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b6910e8 commit a4c3746

20 files changed

Lines changed: 636 additions & 57 deletions

composer.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@
5252
"LTS\\PHPQA\\Tests\\": [
5353
"tests/"
5454
],
55+
"LTS\\PHPQA\\Tests\\Assets\\PHPStan\\ApiInternalLeak\\": [
56+
"tests/assets/PHPStan/ApiInternalLeak/"
57+
],
5558
"LTS\\PHPQA\\Tests\\Assets\\PHPStan\\ApiOrInternal\\": [
5659
"tests/assets/PHPStan/ApiOrInternal/"
5760
],

docs/tools/requireApiOrInternal.md

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,33 @@ genuinely need it *and* the library is willing to support it long-term. A small,
5151
deliberate `@api` surface (often a single facade) is the goal; everything behind
5252
it stays `@internal` and free to evolve.
5353

54-
## Exempting generated / managed code
54+
## Dev-only code is skipped automatically
5555

56-
Generated and managed trees cannot carry hand-authored tags. Exempt them by
57-
fully-qualified namespace **prefix** in your `qaConfig/phpstan.neon`:
56+
Code under an **`autoload-dev`** PSR-4 namespace (tests, dev/maintainer tooling,
57+
QA config) is never installed in a production/runtime install, so it forms no
58+
consumer contract and has nothing to classify. The rule reads your
59+
`composer.json` `autoload-dev.psr-4` prefixes and skips any class-like under one
60+
**no configuration, and you never hand-tag your own test suite.**
61+
62+
## Exempting runtime generated / managed code
63+
64+
A generated or managed tree that lives under the **runtime** `autoload` namespace
65+
(so the dev skip above does not cover it) yet cannot carry hand-authored tags is
66+
exempted by fully-qualified namespace **prefix** in your `qaConfig/phpstan.neon`:
5867

5968
```neon
6069
parameters:
6170
phpqaciApiOrInternal:
6271
ignoredNamespacePrefixes:
6372
- App\Generated
64-
- App\PhpQaCi
6573
```
6674

67-
The default is an empty list (nothing exempt).
75+
The default is an empty list. Prefer to **classify** generated code where you can
76+
— e.g. have your generator stamp `@internal` on every emitted class-like — so the
77+
rule keeps enforcing over it rather than skipping a whole tree. (php-qa-ci's own
78+
[managed source](../../CLAUDE/managed-source.md) does exactly this: the
79+
`FactorySealedBy` artefact it writes into your `<RootNs>\PhpQaCi\` namespace is
80+
generated already carrying `@internal`, so no exemption is needed.)
6881

6982
## How to fix a violation
7083

@@ -92,6 +105,28 @@ conscious rather than inherited from Composer's silent default. A package that i
92105
really an application sets `type: project` (and this rule then no-ops); a real
93106
library sets `type: library` and classifies its surface.
94107

108+
## Coherence: an `@api` class must not expose `@internal` (default-on)
109+
110+
Classifying every class-like is necessary but not sufficient — the classification
111+
can still be **incoherent**. If an `@api` class exposes an `@internal` one through
112+
its public signature (a public method's parameter or return type, or a public
113+
property), a consumer using the supported `@api` type is forced to touch the
114+
`@internal` one and PHPStan flags them with `class.internal` / `method.internal`.
115+
The `@api` promise is hollow.
116+
117+
A second **default-on** rule, `phpqaci.apiMustNotExposeInternal`, catches that
118+
leak at the library's own gate. For a `type: library` project it scans every
119+
`@api` class-like's public surface and errors if a referenced class-like **in the
120+
same package** (same root namespace segment — the boundary PHPStan keys
121+
`@internal` on) is `@internal`. Third-party `@internal` types (a different root
122+
namespace) are not flagged — that is the other package's concern.
123+
124+
Fix a violation by either **promoting** the referenced type to `@api` (commit to
125+
it as public contract) or **keeping it off the public surface** — e.g. map it to
126+
an `@api` DTO at the boundary so the internal type never crosses it. Together the
127+
two rules give *presence* (everything classified) and *coherence* (the public
128+
surface is closed over `@api`).
129+
95130
## Enforcing the boundary in consumers
96131

97132
Classifying the surface declares the contract; it does not stop a *consumer* from
@@ -124,9 +159,14 @@ Any consumer class that depends on a listed internal namespace then fails
124159
## Design / implementation notes
125160

126161
- Pure decision core: [`ApiOrInternalTagDetector`](./../../src/PHPStan/Rules/ApiOrInternalTagDetector.php)
127-
(+ `ApiOrInternalTagVerdict`) — unit-tested exhaustively, no PHPStan Scope needed.
162+
(+ `ApiOrInternalTagVerdictEnum`) — unit-tested exhaustively, no PHPStan Scope needed.
128163
- Rule: [`RequireApiOrInternalTagRule`](./../../src/PHPStan/Rules/RequireApiOrInternalTagRule.php)
129164
(hooks `InClassNode`; reads the class docblock for a standalone `@api`/`@internal`
130165
tag).
131166
- Package kind: [`ProjectComposerTypeReader`](./../../src/PackageType/ProjectComposerTypeReader.php)
132167
— injectable seam over the project `composer.json` (mockable in tests).
168+
- Dev-namespace skip: [`DevAutoloadNamespaceReader`](./../../src/PackageType/DevAutoloadNamespaceReader.php)
169+
— reads `autoload-dev.psr-4` prefixes (same injectable-override pattern).
170+
- Coherence rule: [`ApiMustNotExposeInternalRule`](./../../src/PHPStan/Rules/ApiMustNotExposeInternalRule.php)
171+
(hooks `InClassNode`; reflects each `@api` class's public surface and flags
172+
same-package `@internal` references).

qaConfig/composerRequireChecker.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@
2727
"PHPStan\\Analyser\\NodeCallbackInvoker",
2828
"PHPStan\\Analyser\\Scope",
2929
"PHPStan\\Node\\FileNode",
30+
"PHPStan\\Node\\InClassNode",
3031
"PHPStan\\Reflection\\ClassReflection",
3132
"PHPStan\\Reflection\\ReflectionProvider",
3233
"PHPStan\\Rules\\Rule",
34+
"PHPStan\\Type\\Type",
3335
"PHPStan\\Rules\\RuleErrorBuilder",
3436
"PHPStan\\Rules\\IdentifierRuleError",
3537
"Symfony\\Component\\DependencyInjection\\Attribute\\Autoconfigure",

rules-default.neon

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,20 @@ services:
9191
# for any other type the rule no-ops. The collaborators are autowired from the
9292
# services below; ignored namespace prefixes come from the parameter above.
9393
- LTS\PHPQA\PackageType\ProjectComposerTypeReader
94+
- LTS\PHPQA\PackageType\DevAutoloadNamespaceReader
9495
- LTS\PHPQA\PHPStan\Rules\ApiOrInternalTagDetector
9596
-
9697
class: LTS\PHPQA\PHPStan\Rules\RequireApiOrInternalTagRule
9798
arguments:
9899
ignoredNamespacePrefixes: %phpqaciApiOrInternal.ignoredNamespacePrefixes%
99100
tags:
100101
- phpstan.rules.rule
102+
# API-surface integrity (default-on, library-only): an @api class-like must not
103+
# expose an @internal one (same package) through its public signature — else the
104+
# @api promise is hollow and a consumer is forced to touch an @internal type.
105+
# Complements RequireApiOrInternalTagRule (presence) with coherence. The
106+
# ReflectionProvider is autowired by PHPStan core.
107+
-
108+
class: LTS\PHPQA\PHPStan\Rules\ApiMustNotExposeInternalRule
109+
tags:
110+
- phpstan.rules.rule
Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LTS\PHPQA\PHPStan\Rules;
6+
7+
use LTS\PHPQA\PackageType\ProjectComposerTypeReader;
8+
use PhpParser\Node;
9+
use PHPStan\Analyser\Scope;
10+
use PHPStan\Node\InClassNode;
11+
use PHPStan\Reflection\ClassReflection;
12+
use PHPStan\Reflection\ReflectionProvider;
13+
use PHPStan\Rules\Rule;
14+
use PHPStan\Rules\RuleErrorBuilder;
15+
use PHPStan\Type\Type;
16+
17+
/**
18+
* API-surface integrity (the complement to {@see RequireApiOrInternalTagRule}).
19+
*
20+
* Classifying every class-like `@api` / `@internal` is necessary but not
21+
* sufficient: the classification can still be INCOHERENT. If an `@api` class
22+
* exposes an `@internal` one through its public signature — a public method's
23+
* parameter or return type, or a public property — then a consumer using the
24+
* supported `@api` type is forced to touch the `@internal` one, and PHPStan flags
25+
* the consumer with `class.internal` / `method.internal`. The public contract has
26+
* leaked an internal type; the `@api` promise is hollow.
27+
*
28+
* This rule catches that leak at the library's own gate. For a `type: library`
29+
* project, every `@api` class-like's public surface (public-method parameter and
30+
* return types, public-property types) is scanned; referencing an `@internal`
31+
* class-like IN THE SAME PACKAGE (same root namespace segment — the boundary
32+
* PHPStan keys `@internal` on) is an error. The fix is one of: promote the
33+
* referenced type to `@api` (commit to it as public contract), or keep it off the
34+
* public surface (e.g. map it to an `@api` DTO at the boundary).
35+
*
36+
* Third-party `@internal` types (a different root namespace) are NOT flagged here
37+
* — leaking another package's internals is that package's concern, and flagging it
38+
* would punish unavoidable vendor surface. The rule no-ops for any non-library
39+
* package type.
40+
*
41+
* @implements Rule<InClassNode>
42+
*/
43+
final readonly class ApiMustNotExposeInternalRule implements Rule
44+
{
45+
public const string IDENTIFIER = RuleIdentifierInterface::PREFIX . '.apiMustNotExposeInternal';
46+
47+
public function __construct(
48+
private ProjectComposerTypeReader $typeReader,
49+
private ReflectionProvider $reflectionProvider,
50+
) {
51+
}
52+
53+
public function getNodeType(): string
54+
{
55+
return InClassNode::class;
56+
}
57+
58+
/**
59+
* @return list<\PHPStan\Rules\IdentifierRuleError>
60+
*/
61+
public function processNode(Node $node, Scope $scope): array
62+
{
63+
if ('library' !== $this->typeReader->effectiveType()) {
64+
return [];
65+
}
66+
67+
$classReflection = $node->getClassReflection();
68+
if ($classReflection->isAnonymous()) {
69+
return [];
70+
}
71+
72+
// Only the public (@api) surface can leak; an @internal class referencing
73+
// an @internal one is fine.
74+
if (!$this->hasTag($this->docText($node->getOriginalNode()->getDocComment()), 'api')) {
75+
return [];
76+
}
77+
78+
$className = $classReflection->getName();
79+
$ownRoot = $this->rootSegment($className);
80+
81+
$errors = [];
82+
$reported = [];
83+
foreach ($this->publicSurface($classReflection, $scope) as [$where, $type]) {
84+
foreach ($type->getReferencedClasses() as $referenced) {
85+
if ($referenced === $className) {
86+
continue;
87+
}
88+
89+
if (isset($reported[$referenced])) {
90+
continue;
91+
}
92+
93+
if ($this->rootSegment($referenced) !== $ownRoot) {
94+
continue; // only OUR OWN internals are the leak we own
95+
}
96+
97+
if (!$this->reflectionProvider->hasClass($referenced)) {
98+
continue;
99+
}
100+
101+
if (!$this->isInternal($this->reflectionProvider->getClass($referenced))) {
102+
continue;
103+
}
104+
105+
$reported[$referenced] = true;
106+
$errors[] = RuleErrorBuilder::message(\sprintf(
107+
'Class %s is @api but exposes @internal type %s through its public surface (%s). '
108+
. 'A consumer using this @api class would be forced to touch an @internal one '
109+
. '(PHPStan flags that as class.internal/method.internal). Either promote %s to @api '
110+
. '(commit to it as a public contract) or keep it off the public surface '
111+
. '(e.g. map it to an @api type at the boundary).',
112+
$className,
113+
$referenced,
114+
$where,
115+
$referenced,
116+
))->identifier(self::IDENTIFIER)->build();
117+
}
118+
}
119+
120+
return $errors;
121+
}
122+
123+
/**
124+
* Yield [where-label, type] for every type on the class's own public surface:
125+
* public-method parameter + return types (constructor included) and
126+
* public-property types. Inherited members are skipped (a parent classifies
127+
* its own surface).
128+
*
129+
* @return iterable<array{string, Type}>
130+
*/
131+
private function publicSurface(ClassReflection $classReflection, Scope $scope): iterable
132+
{
133+
$native = $classReflection->getNativeReflection();
134+
$className = $classReflection->getName();
135+
136+
foreach ($native->getMethods() as $nativeMethod) {
137+
if (!$nativeMethod->isPublic()) {
138+
continue;
139+
}
140+
141+
if ($nativeMethod->getDeclaringClass()->getName() !== $className) {
142+
continue;
143+
}
144+
145+
$method = $classReflection->getMethod($nativeMethod->getName(), $scope);
146+
foreach ($method->getVariants() as $variant) {
147+
foreach ($variant->getParameters() as $parameter) {
148+
yield [
149+
\sprintf('parameter $%s of method %s()', $parameter->getName(), $nativeMethod->getName()),
150+
$parameter->getType(),
151+
];
152+
}
153+
154+
yield [\sprintf('return type of method %s()', $nativeMethod->getName()), $variant->getReturnType()];
155+
}
156+
}
157+
158+
foreach ($native->getProperties() as $nativeProperty) {
159+
if (!$nativeProperty->isPublic()) {
160+
continue;
161+
}
162+
163+
if ($nativeProperty->getDeclaringClass()->getName() !== $className) {
164+
continue;
165+
}
166+
167+
if (!$classReflection->hasProperty($nativeProperty->getName())) {
168+
continue;
169+
}
170+
171+
$property = $classReflection->getProperty($nativeProperty->getName(), $scope);
172+
yield [\sprintf('property $%s', $nativeProperty->getName()), $property->getReadableType()];
173+
}
174+
}
175+
176+
private function isInternal(ClassReflection $classReflection): bool
177+
{
178+
return $this->hasTag($this->docText($classReflection->getNativeReflection()->getDocComment()), 'internal');
179+
}
180+
181+
private function rootSegment(string $fqcn): string
182+
{
183+
return explode('\\', ltrim($fqcn, '\\'))[0];
184+
}
185+
186+
private function docText(\PhpParser\Comment\Doc|string|false|null $docComment): string
187+
{
188+
if ($docComment instanceof \PhpParser\Comment\Doc) {
189+
return $docComment->getText();
190+
}
191+
192+
return \is_string($docComment) ? $docComment : '';
193+
}
194+
195+
/**
196+
* Matches a standalone `@api` / `@internal` PHPDoc tag, not a longer tag that
197+
* merely starts with it (e.g. `@apiNote`, `@internalRef`).
198+
*/
199+
private function hasTag(string $docText, string $tag): bool
200+
{
201+
return 1 === \Safe\preg_match('/@' . $tag . '(?![\w-])/', $docText);
202+
}
203+
}

src/PHPStan/Rules/ApiOrInternalTagDetector.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
* one of `@api` (supported) or `@internal` (may change) — neither ⇒ Missing,
1919
* both ⇒ Both (contradictory).
2020
* - Any other type (`project` application, `metapackage`, …): no consumer-facing
21-
* API surface, so the classification is not required — always {@see ApiOrInternalTagVerdict::Ok}.
21+
* API surface, so the classification is not required — always {@see ApiOrInternalTagVerdictEnum::Ok}.
2222
*
2323
* The `type` string is normalised (trimmed + lower-cased) because it is
2424
* author-typed free text in composer.json.
@@ -27,20 +27,20 @@ final class ApiOrInternalTagDetector
2727
{
2828
private const string LIBRARY_TYPE = 'library';
2929

30-
public function classify(string $projectType, bool $hasApi, bool $hasInternal): ApiOrInternalTagVerdict
30+
public function classify(string $projectType, bool $hasApi, bool $hasInternal): ApiOrInternalTagVerdictEnum
3131
{
32-
if (self::LIBRARY_TYPE !== \strtolower(\trim($projectType))) {
33-
return ApiOrInternalTagVerdict::Ok;
32+
if (self::LIBRARY_TYPE !== strtolower(trim($projectType))) {
33+
return ApiOrInternalTagVerdictEnum::Ok;
3434
}
3535

3636
if ($hasApi && $hasInternal) {
37-
return ApiOrInternalTagVerdict::Both;
37+
return ApiOrInternalTagVerdictEnum::Both;
3838
}
3939

4040
if (!$hasApi && !$hasInternal) {
41-
return ApiOrInternalTagVerdict::Missing;
41+
return ApiOrInternalTagVerdictEnum::Missing;
4242
}
4343

44-
return ApiOrInternalTagVerdict::Ok;
44+
return ApiOrInternalTagVerdictEnum::Ok;
4545
}
4646
}

src/PHPStan/Rules/ApiOrInternalTagVerdictEnum.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* - {@see self::Both} — it carries BOTH tags: contradictory; a symbol is
1717
* either public API or internal, never both.
1818
*/
19-
enum ApiOrInternalTagVerdict
19+
enum ApiOrInternalTagVerdictEnum
2020
{
2121
case Ok;
2222
case Missing;

0 commit comments

Comments
 (0)