Repository: martin-helmich/typo3-typoscript-lint (php)

Issue:
"Common path prefix" with ">" operator 
Hi everyone,

I just have a question about the "Common path prefix"-Sniff:
While linting the following TypoScript the linter only warns about line 5 & 6 (`bar.baz`) but not about 2 & 3 (`foo.baa`). 

Is this behaivor intended? And if so, why?

foo.typoscript
```
lib {
    foo.baa.10 >
    foo.baa.20 >

    bar.baz.10 = TEXT
    bar.baz.20 = TEXT
}
```

linter output
```
/usr/bin/php vendor/bin/typoscript-lint 
W                                                    [1 / 1, 100%]

Completed with 4 issues

CHECKSTYLE REPORT
=> test/foo.typoscript
   5 Common path prefix "bar" with assignment to "bar.baz.20" in line 6. Consider merging them into a nested assignment.
   5 Common path prefix "bar.baz" with assignment to "bar.baz.20" in line 6. Consider merging them into a nested assignment.
   6 Common path prefix "bar" with assignment to "bar.baz.10" in line 5. Consider merging them into a nested assignment.
   6 Common path prefix "bar.baz" with assignment to "bar.baz.10" in line 5. Consider merging them into a nested assignment.

SUMMARY
4 issues in total. (4 warnings)
```

KamiYang

Pull request:
NestingConsistencySniff should also consider statements other than assignments
Fixes #122

Patch:
diff --git a/src/Linter/Sniff/NestingConsistencySniff.php b/src/Linter/Sniff/NestingConsistencySniff.php
index 6227f15..fefb87d 100644
--- a/src/Linter/Sniff/NestingConsistencySniff.php
+++ b/src/Linter/Sniff/NestingConsistencySniff.php
@@ -6,8 +6,7 @@ use Helmich\TypoScriptLint\Linter\Sniff\Visitor\SniffVisitor;
 
 class NestingConsistencySniff extends AbstractSyntaxTreeSniff
 {
-    /** @var int */
-    private $commonPathPrefixThreshold = 1;
+    private int $commonPathPrefixThreshold = 1;
 
     public function __construct(array $parameters)
     {
diff --git a/src/Linter/Sniff/Visitor/NestingConsistencyVisitor.php b/src/Linter/Sniff/Visitor/NestingConsistencyVisitor.php
index 06ea3db..f00663e 100644
--- a/src/Linter/Sniff/Visitor/NestingConsistencyVisitor.php
+++ b/src/Linter/Sniff/Visitor/NestingConsistencyVisitor.php
@@ -13,10 +13,9 @@ class NestingConsistencyVisitor implements SniffVisitor
 {
 
     /** @var Issue[] */
-    private $issues = [];
+    private array $issues = [];
 
-    /** @var integer */
-    private $commonPathPrefixThreshold;
+    private int $commonPathPrefixThreshold;
 
     public function __construct(int $commonPathPrefixThreshold = 1)
     {
@@ -64,63 +63,65 @@ class NestingConsistencyVisitor implements SniffVisitor
         // Step 2: Discover all plain assignments and determine whether any of them
         // can be moved within one of the nested assignments.
         foreach ($statements as $statement) {
-            if ($statement instanceof Assignment || $statement instanceof NestedAssignment) {
-                $commonPrefixWarnings = [];
-                foreach ($this->getParentObjectPathsForObjectPath($statement->object->relativeName) as $possibleObjectPath) {
-                    if (isset($knownNestedObjectPaths[$possibleObjectPath])) {
-                        $this->issues[] = new Issue(
-                            $statement->sourceLine,
-                            null,
-                            sprintf(
-                                'Assignment to value "%s", altough nested statement for path "%s" exists at line %d.',
-                                $statement->object->relativeName,
-                                $possibleObjectPath,
-                                $knownNestedObjectPaths[$possibleObjectPath]
-                            ),
-                            Issue::SEVERITY_WARNING,
-                            NestingConsistencySniff::class
-                        );
-                    }
+            if (!isset($statement->object)) {
+                continue;
+            }
+            
+            $commonPrefixWarnings = [];
+            foreach ($this->getParentObjectPathsForObjectPath($statement->object->relativeName) as $possibleObjectPath) {
+                if (isset($knownNestedObjectPaths[$possibleObjectPath])) {
+                    $this->issues[] = new Issue(
+                        $statement->sourceLine,
+                        null,
+                        sprintf(
+                            'Operation on value "%s", altough nested statement for path "%s" exists at line %d.',
+                            $statement->object->relativeName,
+                            $possibleObjectPath,
+                            $knownNestedObjectPaths[$possibleObjectPath]
+                        ),
+                        Issue::SEVERITY_WARNING,
+                        NestingConsistencySniff::class
+                    );
+                }
 
-                    $assignmentsWithCommonPrefix = [];
+                $assignmentsWithCommonPrefix = [];
 
-                    foreach ($knownObjectPaths as $key => $line) {
-                        $key = "" . $key;
+                foreach ($knownObjectPaths as $key => $line) {
+                    $key = "" . $key;
 
-                        if ($key !== $statement->object->relativeName && strpos($key, $possibleObjectPath . '.') === 0) {
-                            if (!isset($assignmentsWithCommonPrefix[$key])) {
-                                $assignmentsWithCommonPrefix[$key] = [];
-                            }
-                            $assignmentsWithCommonPrefix[$possibleObjectPath][] = [$key, $line];
+                    if ($key !== $statement->object->relativeName && strpos($key, $possibleObjectPath . '.') === 0) {
+                        if (!isset($assignmentsWithCommonPrefix[$key])) {
+                            $assignmentsWithCommonPrefix[$key] = [];
                         }
+                        $assignmentsWithCommonPrefix[$possibleObjectPath][] = [$key, $line];
                     }
+                }
 
-                    foreach ($assignmentsWithCommonPrefix as $commonPrefix => $lines) {
-                        if (count($lines) < $this->commonPathPrefixThreshold) {
-                            continue;
-                        }
-
-                        $descr = [];
-                        foreach ($lines as $l) {
-                            $descr[] = sprintf('"%s" in line %d', $l[0], $l[1]);
-                        }
+                foreach ($assignmentsWithCommonPrefix as $commonPrefix => $lines) {
+                    if (count($lines) < $this->commonPathPrefixThreshold) {
+                        continue;
+                    }
 
-                        $commonPrefixWarnings[$commonPrefix] = new Issue(
-                            $statement->sourceLine,
-                            null,
-                            sprintf(
-                                'Common path prefix "%s" with %s to %s. Consider merging them into a nested assignment.',
-                                $commonPrefix,
-                                count($lines) === 1 ? 'assignment' : 'assignments',
-                                implode(", ", $descr)
-                            ),
-                            Issue::SEVERITY_WARNING,
-                            NestingConsistencySniff::class
-                        );
+                    $descr = [];
+                    foreach ($lines as $l) {
+                        $descr[] = sprintf('"%s" in line %d', $l[0], $l[1]);
                     }
+
+                    $commonPrefixWarnings[$commonPrefix] = new Issue(
+                        $statement->sourceLine,
+                        null,
+                        sprintf(
+                            'Common path prefix "%s" with %s to %s. Consider merging them into a nested statement.',
+                            $commonPrefix,
+                            count($lines) === 1 ? 'operation' : 'operations',
+                            implode(", ", $descr)
+                        ),
+                        Issue::SEVERITY_WARNING,
+                        NestingConsistencySniff::class
+                    );
                 }
-                $this->issues = array_merge($this->issues, array_values($commonPrefixWarnings));
             }
+            $this->issues = array_merge($this->issues, array_values($commonPrefixWarnings));
         }
     }
 
@@ -143,10 +144,6 @@ class NestingConsistencyVisitor implements SniffVisitor
      * @param Statement[] $statements
      *
      * @return int[][]
-     *
-     * @phan-return array{0:array<string|int,string>,1:array<string|int,string>}
-     *
-     * @psalm-return array{0: array<string, int>, 1: array<string, int>}
      */
     private function getAssignedObjectPathsFromStatementList(array $statements): array
     {
@@ -155,23 +152,25 @@ class NestingConsistencyVisitor implements SniffVisitor
 
         // Step 1: Discover all nested object assignment statements.
         foreach ($statements as $statement) {
-            if ($statement instanceof Assignment || $statement instanceof NestedAssignment) {
-                $knownObjectPaths[$statement->object->relativeName] = $statement->sourceLine;
-                if ($statement instanceof NestedAssignment) {
-                    if (isset($knownNestedObjectPaths[$statement->object->relativeName])) {
-                        $this->issues[] = new Issue(
-                            $statement->sourceLine,
-                            null,
-                            sprintf(
-                                'Multiple nested statements for object path "%s". Consider merging them into one statement.',
-                                $statement->object->relativeName
-                            ),
-                            Issue::SEVERITY_WARNING,
-                            NestingConsistencySniff::class
-                        );
-                    } else {
-                        $knownNestedObjectPaths[$statement->object->relativeName] = $statement->sourceLine;
-                    }
+            if (!isset($statement->object)) {
+                continue;
+            }
+            
+            $knownObjectPaths[$statement->object->relativeName] = $statement->sourceLine;
+            if ($statement instanceof NestedAssignment) {
+                if (isset($knownNestedObjectPaths[$statement->object->relativeName])) {
+                    $this->issues[] = new Issue(
+                        $statement->sourceLine,
+                        null,
+                        sprintf(
+                            'Multiple nested statements for object path "%s". Consider merging them into one statement.',
+                            $statement->object->relativeName
+                        ),
+                        Issue::SEVERITY_WARNING,
+                        NestingConsistencySniff::class
+                    );
+                } else {
+                    $knownNestedObjectPaths[$statement->object->relativeName] = $statement->sourceLine;
                 }
             }
         }


Test patch:
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/ConfigNoCache/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/ConfigNoCache/output.txt
index 3bd2c91..1b0d752 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/ConfigNoCache/output.txt
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/ConfigNoCache/output.txt
@@ -1,2 +1,2 @@
 2;;Setting config.no_cache = 1 is discouraged as it is bad for performance. Consider using USER_INT object instead. Found in path: testPage2.config.no_cache;warning;Helmich\TypoScriptLint\Linter\Sniff\ConfigNoCacheSniff
-8;;Setting config.no_cache = 1 is discouraged as it is bad for performance. Consider using USER_INT object instead. Found in path: testPage.config.no_cache;warning;Helmich\TypoScriptLint\Linter\Sniff\ConfigNoCacheSniff
+8;;Setting config.no_cache = 1 is discouraged as it is bad for performance. Consider using USER_INT object instead. Found in path: testPage.config.no_cache;warning;Helmich\TypoScriptLint\Linter\Sniff\ConfigNoCacheSniff
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/DuplicateAssignment/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/DuplicateAssignment/output.txt
index 345df00..c6090cd 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/DuplicateAssignment/output.txt
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/DuplicateAssignment/output.txt
@@ -1,2 +1,2 @@
 2;;Value of object "foo.bar" is overwritten in line 4.;warning;Helmich\TypoScriptLint\Linter\Sniff\DuplicateAssignmentSniff
-4;;Assignment to value "foo.bar", altough nested statement for path "foo" exists at line 1.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
\ No newline at end of file
+4;;Operation on value "foo.bar", altough nested statement for path "foo" exists at line 1.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/IndentationCondition/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/IndentationCondition/output.txt
index 682f43b..668bbb5 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/IndentationCondition/output.txt
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/IndentationCondition/output.txt
@@ -1,1 +1,1 @@
-2;2;Expected indent of 0 spaces.;warning;Helmich\TypoScriptLint\Linter\Sniff\IndentationSniff
+2;2;Expected indent of 0 spaces.;warning;Helmich\TypoScriptLint\Linter\Sniff\IndentationSniff
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyIntKeys/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyIntKeys/output.txt
index 11a1509..ff39767 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyIntKeys/output.txt
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyIntKeys/output.txt
@@ -1,1 +1,1 @@
-9;;Assignment to value "0.bar", altough nested statement for path "0" exists at line 3.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
\ No newline at end of file
+9;;Operation on value "0.bar", altough nested statement for path "0" exists at line 3.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyNonAssignments/input.typoscript b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyNonAssignments/input.typoscript
new file mode 100644
index 0000000..731f5e8
--- /dev/null
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyNonAssignments/input.typoscript
@@ -0,0 +1,2 @@
+foo.bar >
+foo.baz > 
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyNonAssignments/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyNonAssignments/output.txt
new file mode 100644
index 0000000..f49f258
--- /dev/null
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyNonAssignments/output.txt
@@ -0,0 +1,2 @@
+1;;Common path prefix "foo" with operation to "foo.baz" in line 2. Consider merging them into a nested statement.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
+2;;Common path prefix "foo" with operation to "foo.bar" in line 1. Consider merging them into a nested statement.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyWithConfigurableThreshold/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyWithConfigurableThreshold/output.txt
index 19c816f..bccbf56 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyWithConfigurableThreshold/output.txt
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/NestingConsistencyWithConfigurableThreshold/output.txt
@@ -1,3 +1,3 @@
-4;;Common path prefix "bar" with assignments to "bar.bar" in line 5, "bar.baz" in line 6. Consider merging them into a nested assignment.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
-5;;Common path prefix "bar" with assignments to "bar.foo" in line 4, "bar.baz" in line 6. Consider merging them into a nested assignment.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
-6;;Common path prefix "bar" with assignments to "bar.foo" in line 4, "bar.bar" in line 5. Consider merging them into a nested assignment.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
\ No newline at end of file
+4;;Common path prefix "bar" with operations to "bar.bar" in line 5, "bar.baz" in line 6. Consider merging them into a nested statement.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
+5;;Common path prefix "bar" with operations to "bar.foo" in line 4, "bar.baz" in line 6. Consider merging them into a nested statement.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
+6;;Common path prefix "bar" with operations to "bar.foo" in line 4, "bar.bar" in line 5. Consider merging them into a nested statement.;warning;Helmich\TypoScriptLint\Linter\Sniff\NestingConsistencySniff
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/OperatorWhitespace/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/OperatorWhitespace/output.txt
index 8c7408b..66230ac 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/OperatorWhitespace/output.txt
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/OperatorWhitespace/output.txt
@@ -4,4 +4,4 @@
 5;;No whitespace after object accessor.;warning;Helmich\TypoScriptLint\Linter\Sniff\OperatorWhitespaceSniff
 5;;No whitespace after operator.;warning;Helmich\TypoScriptLint\Linter\Sniff\OperatorWhitespaceSniff
 6;;No whitespace after object accessor.;warning;Helmich\TypoScriptLint\Linter\Sniff\OperatorWhitespaceSniff
-7;;No whitespace after operator.;warning;Helmich\TypoScriptLint\Linter\Sniff\OperatorWhitespaceSniff
+7;;No whitespace after operator.;warning;Helmich\TypoScriptLint\Linter\Sniff\OperatorWhitespaceSniff
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/ParseError/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/ParseError/output.txt
index c9429e2..3344745 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/ParseError/output.txt
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/ParseError/output.txt
@@ -1,1 +1,1 @@
-;;Parse error: Unterminated nested statement!;error;Helmich\TypoScriptParser\Parser\ParseError
\ No newline at end of file
+;0;Parse error: Unterminated nested statement!;error;Helmich\TypoScriptParser\Parser\ParseError
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/TokenizationError/output.txt b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/TokenizationError/output.txt
index ebf2bd2..bc5f848 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/TokenizationError/output.txt
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/Fixtures/TokenizationError/output.txt
@@ -1,1 +1,1 @@
-1;;Tokenization error: Cannot tokenize line "YAY!!!";error;Helmich\TypoScriptParser\Tokenizer\TokenizerException
\ No newline at end of file
+1;0;Tokenization error: Cannot tokenize line "YAY!!!";error;Helmich\TypoScriptParser\Tokenizer\TokenizerException
\ No newline at end of file
diff --git a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/LinterTest.php b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/LinterTest.php
index 53fcb4b..abdc6f0 100644
--- a/tests/functional/Helmich/TypoScriptLint/Tests/Linter/LinterTest.php
+++ b/tests/functional/Helmich/TypoScriptLint/Tests/Linter/LinterTest.php
@@ -73,6 +73,14 @@ class LinterTest extends TestCase
             return $content;
         };
 
+        if (getenv("UPDATE_SNAPSHOTS")) {
+            $output = dirname($typoscriptFile) . '/output.txt';
+            file_put_contents($output, trim($printActualWarnings()));
+
+            $this->markTestIncomplete();
+            return;
+        }
+
         if (count($expectedWarnings) === 0 && count($report->getFiles()) > 0) {
             $this->fail($printActualWarnings());
         }
@@ -95,7 +103,7 @@ class LinterTest extends TestCase
             $outputLines = array_filter($outputLines, 'strlen');
 
             $reports = array_map(
-                function ($line) use ($file) {
+                function ($line) {
                     $values = str_getcsv($line, ';');
                     return new Issue(
                         (int)$values[0],
diff --git a/tests/unit/Helmich/TypoScriptLint/Linter/Sniff/Visitor/NestingConsistencyVisitorTest.php b/tests/unit/Helmich/TypoScriptLint/Linter/Sniff/Visitor/NestingConsistencyVisitorTest.php
index 5aceed0..fa21724 100644
--- a/tests/unit/Helmich/TypoScriptLint/Linter/Sniff/Visitor/NestingConsistencyVisitorTest.php
+++ b/tests/unit/Helmich/TypoScriptLint/Linter/Sniff/Visitor/NestingConsistencyVisitorTest.php
@@ -13,7 +13,6 @@ use PHPUnit\Framework\TestCase;
 
 use function PHPUnit\Framework\assertThat;
 use function PHPUnit\Framework\assertEquals;
-use function PHPUnit\Framework\equalTo;
 use function PHPUnit\Framework\countOf;
 
 /**
@@ -72,11 +71,11 @@ class NestingConsistencyVisitorTest extends TestCase
 
         assertThat($warnings, countOf(2));
         assertEquals(
-            'Common path prefix "foo" with assignment to "foo.baz" in line 2. Consider merging them into a nested assignment.',
+            'Common path prefix "foo" with operation to "foo.baz" in line 2. Consider merging them into a nested statement.',
             $warnings[0]->getMessage()
         );
         assertEquals(
-            'Common path prefix "foo" with assignment to "foo.bar" in line 1. Consider merging them into a nested assignment.',
+            'Common path prefix "foo" with operation to "foo.bar" in line 1. Consider merging them into a nested statement.',
             $warnings[1]->getMessage()
         );
     }
@@ -115,7 +114,7 @@ class NestingConsistencyVisitorTest extends TestCase
 
         assertThat($warnings, countOf(1));
         assertEquals(
-            'Assignment to value "foo.baz", altough nested statement for path "foo" exists at line 1.',
+            'Operation on value "foo.baz", altough nested statement for path "foo" exists at line 1.',
             $warnings[0]->getMessage()
         );
     }