refactor: centralised source range construction

Author: NickSdot

src/Sniff/AbstractSniff.php

@@ -69,17 +69,15 @@ protected function elementNameRanges(File $file, int $beginOffset, int $untilOff
         $elementNameLength = strlen($elementName);
 
         return [
-            new SourceRange(
-                $file->lineAtOffset($openingNameOffset)->number,
+            SourceRange::fromFile(
+                $file,
                 $openingNameOffset,
                 $openingNameOffset + $elementNameLength,
-                $elementName,
             ),
-            new SourceRange(
-                $file->lineAtOffset($closingNameOffset)->number,
+            SourceRange::fromFile(
+                $file,
                 $closingNameOffset,
                 $closingNameOffset + $elementNameLength,
-                $elementName,
             ),
         ];
     }

src/Sniff/AttributeOrderSniff.php

@@ -17,9 +17,9 @@
  */
 final class AttributeOrderSniff extends AbstractSniff implements Fixable
 {
-    private const string REPORTING_MESSAGE = 'Element <%s>: xml:id should appear before xmlns attributes.';
     private const string OPENING_TAG_PATTERN = '/<([a-zA-Z0-9:_-]+)\b([^<>]*?)>/';
     private const string ATTRIBUTE_NAME_PATTERN = '/([a-zA-Z0-9:_-]+)\s*=/';
+    private const string REPORTING_MESSAGE = 'Element <%s>: xml:id should appear before xmlns attributes.';
 
     public static function getCode(): string
     {
@@ -65,11 +65,10 @@ public function process(\DOMDocument $document, File $file): array
                 $tagName,
                 $attrString,
                 $file->path,
-                new SourceRange(
-                    $file->lineAtOffset($beginOffset)->number,
+                SourceRange::fromFile(
+                    $file,
                     $beginOffset,
                     $beginOffset + strlen($fullMatch),
-                    $fullMatch,
                 ),
                 $violations,
             );

src/Sniff/MixedIndentationSniff.php

@@ -23,7 +23,10 @@ public static function fixerClassName(): string
         return MixedIndentationFixer::class;
     }
 
-    /** @throws \InvalidArgumentException if a generated source range is inconsistent */
+    /**
+     * @throws \InvalidArgumentException if a generated source range is inconsistent
+     * @throws \OutOfBoundsException if a generated source range lies outside the source
+     */
     public function process(\DOMDocument $document, File $file): array
     {
         $violations = [];
@@ -42,11 +45,10 @@ public function process(\DOMDocument $document, File $file): array
                 $file->path,
                 self::REPORTING_MESSAGE,
                 [
-                    new SourceRange(
-                        $line->number,
+                    SourceRange::fromFile(
+                        $file,
                         $line->beginOffset,
                         $line->beginOffset + strlen($indentation),
-                        $indentation,
                     ),
                 ],
             );

src/Sniff/TrailingWhitespaceSniff.php

@@ -23,7 +23,10 @@ public static function fixerClassName(): string
         return TrailingWhitespaceFixer::class;
     }
 
-    /** @throws \InvalidArgumentException if a generated source range is inconsistent */
+    /**
+     * @throws \InvalidArgumentException if a generated source range is inconsistent
+     * @throws \OutOfBoundsException if a generated source range lies outside the source
+     */
     public function process(\DOMDocument $document, File $file): array
     {
         $violations = [];
@@ -39,7 +42,7 @@ public function process(\DOMDocument $document, File $file): array
             $violations[] = $this->createViolation(
                 $file->path,
                 self::REPORTING_MESSAGE,
-                [new SourceRange($line->number, $beginOffset, $beginOffset + strlen($whitespace), $whitespace)],
+                [SourceRange::fromFile($file, $beginOffset, $beginOffset + strlen($whitespace))],
             );
         }
 

src/Sniff/WhitespaceSniff.php

@@ -32,12 +32,12 @@ public static function fixerClassName(): string
     /**
      * @throws \InvalidArgumentException if a generated source range is inconsistent
      * @throws \LogicException if source content cannot be split into lines
+     * @throws \OutOfBoundsException if a generated source range lies outside the source
      */
     public function process(\DOMDocument $document, File $file): array
     {
         $violations = [];
         $offset = 0;
-        $line = 1;
 
         $lines = preg_split(self::LINE_ENDING_PATTERN, $file->content, -1, PREG_SPLIT_DELIM_CAPTURE);
         if ($lines === false) {
@@ -59,12 +59,11 @@ public function process(\DOMDocument $document, File $file): array
                 $violations[] = $this->createViolation(
                     $file->path,
                     $message,
-                    [new SourceRange($line, $offset, $offset + $lineContentLength, $lineContent)],
+                    [SourceRange::fromFile($file, $offset, $offset + $lineContentLength)],
                 );
             }
 
             $offset += $lineContentLength + strlen($lineEnding);
-            $line++;
         }
 
         return $violations;

src/Violation/SourceRange.php

@@ -4,8 +4,24 @@
 
 namespace DocbookCS\Violation;
 
+use DocbookCS\Source\File;
+
 final readonly class SourceRange
 {
+    /**
+     * @throws \InvalidArgumentException if the source range is inconsistent
+     * @throws \OutOfBoundsException if the begin offset lies outside the source
+     */
+    public static function fromFile(File $file, int $beginOffset, int $untilOffset): self
+    {
+        return new self(
+            line: $file->lineAtOffset($beginOffset)->number,
+            beginOffset: $beginOffset,
+            untilOffset: $untilOffset,
+            content: substr($file->content, $beginOffset, $untilOffset - $beginOffset),
+        );
+    }
+
     /** @throws \InvalidArgumentException if the source range is inconsistent */
     public function __construct(
         public int $line,

tests/Unit/Fix/WhitespaceFixerTest.php

@@ -12,6 +12,7 @@
 use DocbookCS\Runner\RunMode;
 use DocbookCS\Sniff\WhitespaceSniff;
 use DocbookCS\Source\File;
+use DocbookCS\Source\Line;
 use DocbookCS\Violation\SourceRange;
 use DocbookCS\Violation\Violation;
 use PHPUnit\Framework\Attributes\CoversClass;
@@ -30,6 +31,7 @@
     //
     UsesClass(File::class),
     UsesClass(FixPlan::class),
+    UsesClass(Line::class),
     UsesClass(SourceRange::class),
 ]
 final class WhitespaceFixerTest extends TestCase

tests/Unit/Sniff/WhitespaceSniffTest.php

@@ -6,6 +6,7 @@
 
 use DocbookCS\Sniff\WhitespaceSniff;
 use DocbookCS\Source\File;
+use DocbookCS\Source\Line;
 use DocbookCS\Violation\SourceRange;
 use DocbookCS\Violation\Violation;
 use PHPUnit\Framework\Attributes\CoversClass;
@@ -18,6 +19,7 @@
     CoversClass(WhitespaceSniff::class),
     //
     UsesClass(File::class),
+    UsesClass(Line::class),
     UsesClass(SourceRange::class),
 ]
 final class WhitespaceSniffTest extends TestCase

tests/Unit/Violation/AffectedRangesTest.php

@@ -4,20 +4,41 @@
 
 namespace DocbookCS\Tests\Unit\Violation;
 
-use DocbookCS\Violation\SourceRange;
+use DocbookCS\Source\File;
+use DocbookCS\Source\Line;
 use DocbookCS\Violation\Severity;
+use DocbookCS\Violation\SourceRange;
 use DocbookCS\Violation\Violation;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\UsesClass;
 use PHPUnit\Framework\TestCase;
 
 #[
     CoversClass(SourceRange::class),
     CoversClass(Violation::class),
+    //
+    UsesClass(File::class),
+    UsesClass(Line::class),
 ]
 final class AffectedRangesTest extends TestCase
 {
+    #[Test]
+    public function itCreatesSourceRangesFromFileOffsets(): void
+    {
+        $range = SourceRange::fromFile(
+            new File('file.xml', "first\nsecond\n"),
+            6,
+            12,
+        );
+
+        self::assertSame(2, $range->line);
+        self::assertSame(6, $range->beginOffset);
+        self::assertSame(12, $range->untilOffset);
+        self::assertSame('second', $range->content);
+    }
+
     #[Test]
     public function itRejectsViolationsWithoutAffectedRanges(): void
     {