diff --git a/docbookcs.xml.dist b/docbookcs.xml.dist index 51ee17a..e4c3370 100644 --- a/docbookcs.xml.dist +++ b/docbookcs.xml.dist @@ -16,6 +16,10 @@ + + + + diff --git a/src/Fix/Fixer/Fixer.php b/src/Fix/Fixer/Fixer.php index a488c7f..8ddefdb 100644 --- a/src/Fix/Fixer/Fixer.php +++ b/src/Fix/Fixer/Fixer.php @@ -9,8 +9,12 @@ use DocbookCS\Fix\FixerException; use DocbookCS\Violation\Violation; +/** @template TFixerData = mixed */ interface Fixer { - /** @throws FixerException */ + /** + * @param Violation $violation + * @throws FixerException + */ public function process(Violation $violation): Fix|FixPlan; } diff --git a/src/Fix/Fixer/MixedUnionFixer.php b/src/Fix/Fixer/MixedUnionFixer.php new file mode 100644 index 0000000..f809579 --- /dev/null +++ b/src/Fix/Fixer/MixedUnionFixer.php @@ -0,0 +1,34 @@ + */ +final class MixedUnionFixer implements Fixer +{ + /** + * @param Violation $violation + * @throws FixerException + */ + public function process(Violation $violation): Fix + { + $range = $violation->rangeOne(); + if ($range->content === null) { + throw FixerException::cannotFixMissingContent(); + } + + if ( + $violation->fixerData !== 'mixed' + || preg_match('/^]*\bclass\s*=\s*(["\'])union\1[^>]*>.*<\/type>$/is', $range->content) !== 1 + ) { + throw FixerException::cannotFixInvalidContent($violation); + } + + return Fix::fromViolationAndRange($violation, $range, $violation->fixerData); + } +} diff --git a/src/Fix/Fixer/NativeTypeFixer.php b/src/Fix/Fixer/NativeTypeFixer.php new file mode 100644 index 0000000..17067a9 --- /dev/null +++ b/src/Fix/Fixer/NativeTypeFixer.php @@ -0,0 +1,31 @@ + */ +final class NativeTypeFixer implements Fixer +{ + /** + * @param Violation $violation + * @throws FixerException + */ + public function process(Violation $violation): Fix + { + $range = $violation->rangeOne(); + if ($range->content === null) { + throw FixerException::cannotFixMissingContent(); + } + + if (!is_string($violation->fixerData) || preg_match('/^[a-z]+$/', $violation->fixerData) !== 1) { + throw FixerException::cannotFixInvalidContent($violation); + } + + return Fix::fromViolationAndRange($violation, $range, $violation->fixerData); + } +} diff --git a/src/Sniff/AbstractSniff.php b/src/Sniff/AbstractSniff.php index 8bc8997..25763f2 100644 --- a/src/Sniff/AbstractSniff.php +++ b/src/Sniff/AbstractSniff.php @@ -10,6 +10,10 @@ use DocbookCS\Violation\SourceRange; use DocbookCS\Violation\Violation; +/** + * @template TFixerData = mixed + * @implements SniffInterface + */ abstract class AbstractSniff implements SniffInterface { protected Severity $severity = Severity::ERROR; @@ -72,17 +76,24 @@ protected function elementNameRanges(File $file, int $beginOffset, int $untilOff /** * @param non-empty-list $affectedRanges + * @param TFixerData $fixerData * + * @return Violation * @throws \InvalidArgumentException if the affected ranges are inconsistent */ - protected function createViolation(string $filePath, string $message, array $affectedRanges): Violation - { + protected function createViolation( + string $filePath, + string $message, + array $affectedRanges, + mixed $fixerData = null, + ): Violation { return new Violation( sniffCode: static::getCode(), filePath: $filePath, message: $message, affectedRanges: $affectedRanges, severity: $this->severity, + fixerData: $fixerData, ); } } diff --git a/src/Sniff/Fixable.php b/src/Sniff/Fixable.php index 270bfab..d21ce0a 100644 --- a/src/Sniff/Fixable.php +++ b/src/Sniff/Fixable.php @@ -6,8 +6,12 @@ use DocbookCS\Fix\Fixer\Fixer; +/** + * @template TFixerData = mixed + * @extends SniffInterface + */ interface Fixable extends SniffInterface { - /** @return class-string */ + /** @return class-string> */ public static function getFixerClassName(): string; } diff --git a/src/Sniff/MixedUnionDetector.php b/src/Sniff/MixedUnionDetector.php new file mode 100644 index 0000000..d996ec1 --- /dev/null +++ b/src/Sniff/MixedUnionDetector.php @@ -0,0 +1,56 @@ + */ + public static function matches(string $source): array + { + preg_match_all('/<\/?type\b[^>]*>/i', $source, $matches, PREG_OFFSET_CAPTURE); + /** @var list}> $stack */ + $stack = []; + $unions = []; + + foreach ($matches[0] as [$tag, $offset]) { + $offset = (int) $offset; + if (!str_starts_with($tag, '')) { + if ($stack !== [] && $stack[array_key_last($stack)]['union']) { + $stack[array_key_last($stack)]['members'][] = ''; + } + continue; + } + + $stack[] = [ + 'beginOffset' => $offset, + 'contentOffset' => $offset + strlen($tag), + 'union' => preg_match('/\bclass\s*=\s*(["\'])union\1/i', $tag) === 1, + 'members' => [], + ]; + continue; + } + + if (null === $type = array_pop($stack)) { + continue; + } + + $content = trim(substr($source, $type['contentOffset'], $offset - $type['contentOffset'])); + if ($type['union']) { + $members = array_map('strtolower', $type['members']); + if (count($members) >= 2 && in_array('mixed', $members, true)) { + $unions[] = [ + 'beginOffset' => $type['beginOffset'], + 'untilOffset' => $offset + strlen($tag), + ]; + } + } elseif ($stack !== [] && $stack[array_key_last($stack)]['union']) { + $stack[array_key_last($stack)]['members'][] = $content; + } + } + + return $unions; + } +} diff --git a/src/Sniff/MixedUnionSniff.php b/src/Sniff/MixedUnionSniff.php new file mode 100644 index 0000000..6af373f --- /dev/null +++ b/src/Sniff/MixedUnionSniff.php @@ -0,0 +1,82 @@ + + * @implements Fixable + */ +final class MixedUnionSniff extends AbstractSniff implements Fixable +{ + public static function getCode(): string + { + return 'DocbookCS.MixedUnion'; + } + + public static function getFixerClassName(): string + { + return MixedUnionFixer::class; + } + + /** + * @throws \InvalidArgumentException if a generated source range is inconsistent + * @throws \OutOfBoundsException if a matched union lies outside the source + */ + public function process(\DOMDocument $document, File $file): array + { + $source = $file->contentWithNonElementMarkupMasked(); + $synopsisRanges = $this->synopsisRanges($source); + $violations = []; + + foreach (MixedUnionDetector::matches($source) as $union) { + if (!$this->isInsideSynopsis($union['beginOffset'], $synopsisRanges)) { + continue; + } + + $violations[] = $this->createViolation( + $file->path, + 'A union containing mixed is redundant and should be mixed.', + [SourceRange::fromFile($file, $union['beginOffset'], $union['untilOffset'])], + 'mixed', + ); + } + + return $violations; + } + + /** @return list */ + private function synopsisRanges(string $source): array + { + preg_match_all('/<\/?(?:method|constructor)synopsis\b[^>]*>/i', $source, $matches, PREG_OFFSET_CAPTURE); + $stack = []; + $ranges = []; + foreach ($matches[0] as [$tag, $offset]) { + $offset = (int) $offset; + if (!str_starts_with($tag, ' $beginOffset, 'untilOffset' => $offset + strlen($tag)]; + } + } + + return $ranges; + } + + /** @param list $ranges */ + private function isInsideSynopsis(int $offset, array $ranges): bool + { + foreach ($ranges as $range) { + if ($offset >= $range['beginOffset'] && $offset < $range['untilOffset']) { + return true; + } + } + + return false; + } +} diff --git a/src/Sniff/NativeTypeSniff.php b/src/Sniff/NativeTypeSniff.php new file mode 100644 index 0000000..06254d8 --- /dev/null +++ b/src/Sniff/NativeTypeSniff.php @@ -0,0 +1,145 @@ + + * @implements Fixable + */ +final class NativeTypeSniff extends AbstractSniff implements Fixable +{ + private const array NATIVE_TYPES = [ + 'array', + 'bool', + 'callable', + 'false', + 'float', + 'int', + 'iterable', + 'mixed', + 'never', + 'null', + 'object', + 'resource', + 'string', + 'true', + 'void', + ]; + + private const array TYPE_ALIASES = [ + 'boolean' => 'bool', + 'double' => 'float', + 'integer' => 'int', + 'real' => 'float', + ]; + + public static function getCode(): string + { + return 'DocbookCS.NativeType'; + } + + public static function getFixerClassName(): string + { + return NativeTypeFixer::class; + } + + /** + * @throws \InvalidArgumentException if a generated source range is inconsistent + * @throws \OutOfBoundsException if a matched type lies outside the source + */ + public function process(\DOMDocument $document, File $file): array + { + $source = $file->contentWithNonElementMarkupMasked(); + $synopsisRanges = $this->synopsisRanges($source); + $redundantMixedUnions = array_values(array_filter( + MixedUnionDetector::matches($source), + fn(array $union): bool => $this->isInsideSynopsis($union['beginOffset'], $synopsisRanges), + )); + $violations = []; + + preg_match_all('/]*>([^<]*)<\/type>/i', $source, $matches, PREG_OFFSET_CAPTURE); + foreach ($matches[1] as [$typeName, $offset]) { + $offset = (int) $offset; + if (!$this->isInsideSynopsis($offset, $synopsisRanges)) { + continue; + } + + $canonical = $this->canonicalType(trim($typeName)); + if ($canonical === null || $canonical === trim($typeName)) { + continue; + } + + $leadingWhitespace = strlen($typeName) - strlen(ltrim($typeName)); + $beginOffset = $offset + $leadingWhitespace; + $untilOffset = $beginOffset + strlen(trim($typeName)); + if ($this->isInsideUnion($beginOffset, $untilOffset, $redundantMixedUnions)) { + continue; + } + + $violations[] = $this->createViolation( + $file->path, + sprintf('Native type "%s" should be written as "%s".', trim($typeName), $canonical), + [SourceRange::fromFile($file, $beginOffset, $untilOffset)], + $canonical, + ); + } + + return $violations; + } + + private function canonicalType(string $typeName): ?string + { + $lowercase = strtolower($typeName); + + return self::TYPE_ALIASES[$lowercase] + ?? (in_array($lowercase, self::NATIVE_TYPES, true) ? $lowercase : null); + } + + /** @return list */ + private function synopsisRanges(string $source): array + { + preg_match_all('/<\/?(?:method|constructor)synopsis\b[^>]*>/i', $source, $matches, PREG_OFFSET_CAPTURE); + $stack = []; + $ranges = []; + foreach ($matches[0] as [$tag, $offset]) { + $offset = (int) $offset; + if (!str_starts_with($tag, ' $beginOffset, 'untilOffset' => $offset + strlen($tag)]; + } + } + + return $ranges; + } + + /** @param list $ranges */ + private function isInsideSynopsis(int $offset, array $ranges): bool + { + foreach ($ranges as $range) { + if ($offset >= $range['beginOffset'] && $offset < $range['untilOffset']) { + return true; + } + } + + return false; + } + + /** @param list $unions */ + private function isInsideUnion(int $beginOffset, int $untilOffset, array $unions): bool + { + foreach ($unions as $union) { + if ($beginOffset >= $union['beginOffset'] && $untilOffset <= $union['untilOffset']) { + return true; + } + } + + return false; + } +} diff --git a/src/Sniff/SniffInterface.php b/src/Sniff/SniffInterface.php index bb7743e..c29bbce 100644 --- a/src/Sniff/SniffInterface.php +++ b/src/Sniff/SniffInterface.php @@ -11,6 +11,7 @@ * A sniff receives a DOMDocument (already loaded) and its source file, * then returns zero or more findings for reports and optional fixes. */ +/** @template TFixerData = mixed */ interface SniffInterface { /** @@ -21,7 +22,7 @@ public static function getCode(): string; /** * Apply the sniff to the given document. * - * @return list + * @return list> */ public function process(\DOMDocument $document, File $file): array; diff --git a/src/Violation/Violation.php b/src/Violation/Violation.php index 4b1ef29..a26903d 100644 --- a/src/Violation/Violation.php +++ b/src/Violation/Violation.php @@ -4,6 +4,7 @@ namespace DocbookCS\Violation; +/** @template TFixerData = mixed */ final readonly class Violation { /** @@ -16,6 +17,12 @@ public function __construct( public string $message, public array $affectedRanges, public Severity $severity = Severity::WARNING, + /** + * Data passed from the sniff to its fixer. Fixers are responsible for validating its shape. + * + * @var TFixerData + */ + public mixed $fixerData = null, ) { if ($affectedRanges === []) { throw new \InvalidArgumentException('A violation must affect at least one source range.'); diff --git a/tests/Unit/Fix/MixedUnionFixerTest.php b/tests/Unit/Fix/MixedUnionFixerTest.php new file mode 100644 index 0000000..1dde6e9 --- /dev/null +++ b/tests/Unit/Fix/MixedUnionFixerTest.php @@ -0,0 +1,86 @@ +mixedArray' + . 'example'; + + $file = new File('file.xml', $content); + $document = new \DOMDocument(); + $document->loadXML($content); + $violations = [ + ...new NativeTypeSniff(RunMode::Fix)->process($document, $file), + ...new MixedUnionSniff(RunMode::Fix)->process($document, $file), + ]; + self::assertCount(1, $violations); + self::assertSame('DocbookCS.MixedUnion', $violations[0]->sniffCode); + $fixer = new MixedUnionFixer(); + $result = new FixApplier()->apply($file, array_map($fixer->process(...), $violations)); + + self::assertSame( + 'mixedexample', + $result->file->content, + ); + self::assertSame(1, $result->applied); + } + + #[Test] + public function itRejectsUnexpectedFixerData(): void + { + $content = 'mixedint'; + $violation = new Violation( + 'DocbookCS.MixedUnion', + 'file.xml', + 'Message', + [new SourceRange(1, 0, strlen($content), $content)], + fixerData: ['replacement' => 'mixed'], + ); + + $this->expectException(FixerException::class); + + // Bypass the PHPStan generic contract to exercise the runtime boundary. + new \ReflectionMethod(MixedUnionFixer::class, 'process')->invoke(new MixedUnionFixer(), $violation); + } +} diff --git a/tests/Unit/Fix/NativeTypeFixerTest.php b/tests/Unit/Fix/NativeTypeFixerTest.php new file mode 100644 index 0000000..0d066b7 --- /dev/null +++ b/tests/Unit/Fix/NativeTypeFixerTest.php @@ -0,0 +1,102 @@ +Arrayexample' + . 'integervalue'; + + $result = $this->fix($content); + + self::assertSame( + 'arrayexample' + . 'intvalue', + $result->file->content, + ); + self::assertSame(2, $result->applied); + } + + #[Test] + public function itRejectsNonStringFixerData(): void + { + $violation = new Violation( + 'DocbookCS.NativeType', + 'file.xml', + 'Message', + [new SourceRange(1, 0, 5, 'Array')], + fixerData: ['replacement' => 'array'], + ); + + $this->expectException(FixerException::class); + + // Bypass the PHPStan generic contract to exercise the runtime boundary. + new \ReflectionMethod(NativeTypeFixer::class, 'process')->invoke(new NativeTypeFixer(), $violation); + } + + #[Test] + public function itRejectsAnInvalidReplacement(): void + { + $violation = new Violation( + 'DocbookCS.NativeType', + 'file.xml', + 'Message', + [new SourceRange(1, 0, 5, 'Array')], + fixerData: '', + ); + + $this->expectException(FixerException::class); + + new NativeTypeFixer()->process($violation); + } + + private function fix(string $content): FixResult + { + $file = new File('file.xml', $content); + $document = new \DOMDocument(); + $document->loadXML($content); + $violations = new NativeTypeSniff(RunMode::Fix)->process($document, $file); + $fixer = new NativeTypeFixer(); + $fixes = array_map($fixer->process(...), $violations); + + return new FixApplier()->apply($file, $fixes); + } +} diff --git a/tests/Unit/Runner/SniffRunnerTest.php b/tests/Unit/Runner/SniffRunnerTest.php index 9554e7a..3bafe65 100644 --- a/tests/Unit/Runner/SniffRunnerTest.php +++ b/tests/Unit/Runner/SniffRunnerTest.php @@ -144,7 +144,7 @@ public function itCallsProgressMethods(): void #[Test] public function itAddsFileReportsForFilesWithViolations(): void { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; @@ -181,7 +181,7 @@ public function setProperty(string $name, string $value): void #[Test] public function itStoresAbsolutePathsInFileReports(): void { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; @@ -371,7 +371,7 @@ public function itScansLexicallyEquivalentWideTargetsOnlyOnce(): void #[Test] public function itReportsNoViolationsForFilesInDiffWithoutAddedLines(): void { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { public static function getCode(): string { return 'Test.ViolatingSniff'; diff --git a/tests/Unit/Runner/XmlSniffRunnerTest.php b/tests/Unit/Runner/XmlSniffRunnerTest.php index b31b2fc..ad2a5cb 100644 --- a/tests/Unit/Runner/XmlSniffRunnerTest.php +++ b/tests/Unit/Runner/XmlSniffRunnerTest.php @@ -312,7 +312,7 @@ public function itReportsNoViolationsInDiffModeWhenNoLinesWereAdded(): void #[Test] public function itDoesNotFixViolationsFromNonFixableSniffs(): void { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { public static function getCode(): string { return 'Test.NonFixable'; @@ -347,7 +347,7 @@ public function setProperty(string $name, string $value): void #[Test] public function itThrowsWhenFixableSniffReportsViolationWithoutContentInFixMode(): void { - $sniff = new class implements Fixable { + $sniff = new /** @implements Fixable */ class implements Fixable { public static function getCode(): string { return 'Test.BrokenFixable'; @@ -392,7 +392,7 @@ public function setProperty(string $name, string $value): void /** @param list $lines */ private function sniff(array $lines): SniffInterface { - $sniff = new class implements SniffInterface { + $sniff = new /** @implements SniffInterface */ class implements SniffInterface { /** @var list */ public array $lines = []; diff --git a/tests/Unit/Sniff/MixedUnionSniffTest.php b/tests/Unit/Sniff/MixedUnionSniffTest.php new file mode 100644 index 0000000..f665a3c --- /dev/null +++ b/tests/Unit/Sniff/MixedUnionSniffTest.php @@ -0,0 +1,75 @@ +mixednull' + . 'example'; + + $violations = $this->process($content); + + self::assertCount(1, $violations); + self::assertSame('DocbookCS.MixedUnion', $violations[0]->sniffCode); + self::assertStringContainsString('union containing mixed', $violations[0]->message); + self::assertSame('mixed', $violations[0]->fixerData); + self::assertSame( + 'mixednull', + $violations[0]->rangeOne()->content, + ); + } + + #[Test] + public function itAcceptsAUnionWithoutMixedAndMixedByItself(): void + { + $content = 'stringnull' + . 'examplemixed' + . 'value'; + + self::assertSame([], $this->process($content)); + } + + #[Test] + public function itDoesNotInspectUnionsOutsideSynopsesOrInsideComments(): void + { + $union = 'mixednull'; + $content = '' . $union . 'void' + . 'example'; + + self::assertSame([], $this->process($content)); + } + + /** @return list */ + private function process(string $content): array + { + $document = new \DOMDocument(); + $document->loadXML($content); + + return new MixedUnionSniff()->process($document, new File('file.xml', $content)); + } +} diff --git a/tests/Unit/Sniff/NativeTypeSniffTest.php b/tests/Unit/Sniff/NativeTypeSniffTest.php new file mode 100644 index 0000000..3b9bf78 --- /dev/null +++ b/tests/Unit/Sniff/NativeTypeSniffTest.php @@ -0,0 +1,101 @@ +stringexample' + . 'ExampleClassvalue'; + + self::assertSame([], $this->process($content)); + } + + #[Test] + public function itReportsNonCanonicalCasingAndAliases(): void + { + $content = 'Arrayexample' + . 'integervalue' + . 'BOOLEANenabled'; + + $violations = $this->process($content); + + self::assertCount(3, $violations); + self::assertSame('array', $this->replacementFromMessage($violations[0]->message)); + self::assertSame('int', $this->replacementFromMessage($violations[1]->message)); + self::assertSame('bool', $this->replacementFromMessage($violations[2]->message)); + self::assertSame('Array', $violations[0]->rangeOne()->content); + self::assertSame('array', $violations[0]->fixerData); + } + + #[Test] + public function itDefersMembersOfRedundantMixedUnionsToTheMixedUnionSniff(): void + { + $content = 'mixedArray' + . 'example'; + + self::assertSame([], $this->process($content)); + } + + #[Test] + public function itDoesNotInspectTypeElementsOutsideSynopsesOrInsideComments(): void + { + $content = 'String' + . '' + . 'voidexample'; + + self::assertSame([], $this->process($content)); + } + + #[Test] + public function itReportsInSourceOrder(): void + { + $content = "\n Stringexample\n" + . " Callablevalue\n"; + + $violations = $this->process($content); + + self::assertCount(2, $violations); + self::assertSame(2, $violations[0]->rangeOne()->line); + self::assertSame(3, $violations[1]->rangeOne()->line); + } + + /** @return list */ + private function process(string $content): array + { + $document = new \DOMDocument(); + $document->loadXML($content); + + return new NativeTypeSniff()->process($document, new File('file.xml', $content)); + } + + private function replacementFromMessage(string $message): string + { + self::assertSame(1, preg_match('/should be written as "([a-z]+)"/', $message, $matches)); + + return $matches[1]; + } +} diff --git a/tests/Unit/Violation/AffectedRangesTest.php b/tests/Unit/Violation/AffectedRangesTest.php index d9c050d..e44b84c 100644 --- a/tests/Unit/Violation/AffectedRangesTest.php +++ b/tests/Unit/Violation/AffectedRangesTest.php @@ -57,6 +57,22 @@ public function itRejectsUnorderedOrOverlappingAffectedRanges(): void ]); } + #[Test] + public function itTransportsArbitraryFixerData(): void + { + $fixerData = ['replacement' => 'int', 'attributes' => ['role' => 'return']]; + + $violation = new Violation( + 'Test', + 'file.xml', + 'Message', + [new SourceRange(1, 0, 1)], + fixerData: $fixerData, + ); + + self::assertSame($fixerData, $violation->fixerData); + } + #[Test, DataProvider('invalidSourceRanges')] public function itRejectsInvalidSourceRanges(int $line, int $beginOffset, int $untilOffset, ?string $content): void {