[TASK] Warn about empty @import in TypoScript

When @import or <INCLUDE_TYPOSCRIPT: lines do not
resolve to any include, this probably indicates
a broken import statement.

To help integrators find and fix those, the
Page TSconfig and TypoScript backend modules now
list them as "Syntax scanner warnings".

We're adding a functional test to verify the detection
works. This will ensure it does not break when we refactor
some details of the include tree structure later. Doing as
described in #102102 and #102103 would simplify and
streamline various details, but it's currently unclear if
we should still do this in v12.

Resolves: #102064
Related: #100218
Related: #97816
Related: #102102
Related: #102103
Releases: main, 12.4
Change-Id: I1bba0cead97ff81b38c97f18581275edee01ea63
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/81358
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>

typo3/sysext/backend/Resources/Private/Language/locallang_pagetsconfig.xlf

@@ -135,6 +135,9 @@
 			<trans-unit id="module.pagetsconfig_includes.syntaxError.type.brace.missing" resname="module.pagetsconfig_includes.syntaxError.type.brace.missing">
 				<source>Brace missing in "%1$s", line number "%2$s"</source>
 			</trans-unit>
+			<trans-unit id="module.pagetsconfig_includes.syntaxError.type.import.empty" resname="module.pagetsconfig_includes.syntaxError.type.import.empty">
+				<source>Import does not find a file in "%1$s", line number "%2$s"</source>
+			</trans-unit>
 			<trans-unit id="module.pagetsconfig_includes.tree.child.type.Segment" resname="module.pagetsconfig_includes.tree.child.type.Segment">
 				<source>Code segment</source>
 			</trans-unit>

typo3/sysext/core/Classes/TypoScript/IncludeTree/Visitor/IncludeTreeSyntaxScannerVisitor.php

@@ -17,9 +17,16 @@
 
 namespace TYPO3\CMS\Core\TypoScript\IncludeTree\Visitor;
 
+use TYPO3\CMS\Core\TypoScript\IncludeTree\IncludeNode\AtImportInclude;
+use TYPO3\CMS\Core\TypoScript\IncludeTree\IncludeNode\ConditionElseInclude;
+use TYPO3\CMS\Core\TypoScript\IncludeTree\IncludeNode\ConditionInclude;
+use TYPO3\CMS\Core\TypoScript\IncludeTree\IncludeNode\ConditionIncludeTyposcriptInclude;
 use TYPO3\CMS\Core\TypoScript\IncludeTree\IncludeNode\IncludeInterface;
+use TYPO3\CMS\Core\TypoScript\IncludeTree\IncludeNode\IncludeTyposcriptInclude;
 use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\BlockCloseLine;
 use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\IdentifierBlockOpenLine;
+use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\ImportLine;
+use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\ImportOldLine;
 use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\InvalidLine;
 use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\LineInterface;
 
@@ -32,12 +39,12 @@
 final class IncludeTreeSyntaxScannerVisitor implements IncludeTreeVisitorInterface
 {
     /**
-     * @var list<array{type: string, include: IncludeInterface, line: LineInterface}>
+     * @var list<array{type: string, include: IncludeInterface, line: LineInterface, lineNumber: int}>
      */
     private array $errors = [];
 
     /**
-     * @return list<array{type: string, include: IncludeInterface, line: LineInterface}>
+     * @return list<array{type: string, include: IncludeInterface, line: LineInterface, lineNumber: int}>
      */
     public function getErrors(): array
     {
@@ -49,6 +56,27 @@ public function visitBeforeChildren(IncludeInterface $include, int $currentDepth
     }
 
     public function visit(IncludeInterface $include, int $currentDepth): void
+    {
+        $this->brokenLinesAndBraces($include);
+        $this->emptyImports($include);
+
+        // Add the line number of the first token of the line object to the error array.
+        // Not strictly needed, but more convenient in Fluid template to render.
+        foreach ($this->errors as &$error) {
+            /** @var LineInterface $line */
+            $line = $error['line'];
+            $error['lineNumber'] = $line->getTokenStream()->reset()->peekNext()->getLine();
+        }
+
+        // Sort array by line number to list them top->bottom in view.
+        usort($this->errors, fn ($a, $b) => $a['lineNumber'] <=> $b['lineNumber']);
+    }
+
+    /**
+     * Scan for invalid lines ("foo.bar <" is invalid since there must be something after "<"),
+     * and scan for "too many" and "not enough" "}" braces.
+     */
+    private function brokenLinesAndBraces(IncludeInterface $include): void
     {
         if ($include->isSplit()) {
             // If this node is split, don't check for syntax errors, this is
@@ -92,13 +120,67 @@ public function visit(IncludeInterface $include, int $currentDepth): void
                 'line' => $lastLine,
             ];
         }
+    }
 
-        // Add the line number of the first token of the line object to the error array.
-        // Not strictly needed, but more convenient in Fluid template to render.
-        foreach ($this->errors as &$error) {
-            /** @var LineInterface $line */
-            $line = $error['line'];
-            $error['lineNumber'] = $line->getTokenStream()->reset()->peekNext()->getLine();
+    /**
+     * Look for @import and INCLUDE_TYPOSCRIPT that don't find to-include file(s).
+     *
+     * @todo: This code is far more complex than it could be. See #102102 and #102103 for
+     *        changes we should apply to the include tree structure to simplify this.
+     */
+    private function emptyImports(IncludeInterface $include): void
+    {
+        if (!$include->isSplit()) {
+            // Nodes containing @import are always split
+            return;
+        }
+        $lineStream = $include->getLineStream();
+        if (!$lineStream) {
+            // A node that is split should never have an empty line stream,
+            // this may be obsolete, but does not hurt much.
+            return;
+        }
+        // Find @import lines in this include, index by
+        // combination of line number and column position.
+        $allImportLines = [];
+        foreach ($lineStream->getNextLine() as $line) {
+            if ($line instanceof ImportLine || $line instanceof ImportOldLine) {
+                $valueToken = $line->getValueToken();
+                $allImportLines[$valueToken->getLine() . '-' . $valueToken->getColumn()] = $line;
+            }
+        }
+        // Now iterate children to exclude valid allImportLines, those that included something.
+        foreach ($include->getNextChild() as $child) {
+            if ($child instanceof AtImportInclude || $child instanceof IncludeTyposcriptInclude) {
+                /** @var ImportLine|ImportOldLine $originalLine */
+                $originalLine = $child->getOriginalLine();
+                $valueToken = $originalLine->getValueToken();
+                unset($allImportLines[$valueToken->getLine() . '-' . $valueToken->getColumn()]);
+            }
+            // Condition includes don't have the "body" lines itself (or a "body" sub node). This may change,
+            // but until then we'll have to scan the parent node and loop condition includes here to find out
+            // which of them resolved to child nodes.
+            if ($child instanceof ConditionInclude
+                || $child instanceof ConditionElseInclude
+                || $child instanceof ConditionIncludeTyposcriptInclude
+            ) {
+                foreach ($child->getNextChild() as $conditionChild) {
+                    if ($conditionChild instanceof AtImportInclude || $conditionChild instanceof IncludeTyposcriptInclude) {
+                        /** @var ImportLine|ImportOldLine $originalLine */
+                        $originalLine = $conditionChild->getOriginalLine();
+                        $valueToken = $originalLine->getValueToken();
+                        unset($allImportLines[$valueToken->getLine() . '-' . $valueToken->getColumn()]);
+                    }
+                }
+            }
+        }
+        // Everything left are invalid includes
+        foreach ($allImportLines as $importLine) {
+            $this->errors[] = [
+                'type' => 'import.empty',
+                'include' => $include,
+                'line' => $importLine,
+            ];
         }
     }
 }

typo3/sysext/core/Documentation/Changelog/12.0/Feature-97816-TypoScriptSyntaxImprovements.rst

@@ -70,9 +70,9 @@ syntax, and integrators are encouraged to fully switch to :typoscript:`@import`.
 ..  code-block:: typoscript
 
     [frontend.user.isLoggedIn]
-      @import 'EXT:my_extension/Configuration/TypoScript/LoggedInUser.typoscript
+      @import 'EXT:my_extension/Configuration/TypoScript/LoggedInUser.typoscript'
     [ELSE]
-      @import 'EXT:my_extension/Configuration/TypoScript/NotLoggedInUser.typoscript
+      @import 'EXT:my_extension/Configuration/TypoScript/NotLoggedInUser.typoscript'
     [END]
 
 Scope restriction to file / snipped level
@@ -96,9 +96,9 @@ two conditions follow directly in one snippet:
 ..  code-block:: typoscript
 
     [frontend.user.isLoggedIn]
-      @import 'EXT:my_extension/Configuration/TypoScript/LoggedInUser.typoscript
+      @import 'EXT:my_extension/Configuration/TypoScript/LoggedInUser.typoscript'
     [applicationContext == "Development"]
-      @import 'EXT:my_extension/Configuration/TypoScript/Development.typoscript
+      @import 'EXT:my_extension/Configuration/TypoScript/Development.typoscript'
     [END]
 
 This always worked and did not change with the new parser: Opening a new condition
@@ -113,12 +113,12 @@ included if a user is logged in *and* the application is in development context.
 ..  code-block:: typoscript
 
     [frontend.user.isLoggedIn]
-      @import 'EXT:my_extension/Configuration/TypoScript/LoggedInUser.typoscript
+      @import 'EXT:my_extension/Configuration/TypoScript/LoggedInUser.typoscript'
     [END]
 
     # File LoggedInUser.typoscript:
     [applicationContext == "Development"]
-      @import 'EXT:my_extension/Configuration/TypoScript/LoggedInUserDevelopment.typoscript
+      @import 'EXT:my_extension/Configuration/TypoScript/LoggedInUserDevelopment.typoscript'
     [END]
 
 Irrelevant order of <INCLUDE_TYPOSCRIPT: tokens

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/Imports/validImportA.typoscript

@@ -0,0 +1 @@
+validImportA = validImportA

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/Imports/validImportB1.typoscript

@@ -0,0 +1 @@
+validImportB1 = validImportB1

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/Imports/validImportB2.typoscript

@@ -0,0 +1 @@
+validImportB2 = validImportB2

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/Imports/validImportC.typoscript

@@ -0,0 +1 @@
+validImportC = validImportC

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/Imports/validImportD1.typoscript

@@ -0,0 +1 @@
+validImportD1 = validImportD1

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/Imports/validImportD2.typoscript

@@ -0,0 +1 @@
+validImportD1 = validImportD1

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/Imports/validIncludeTypoScriptA.typoscript

@@ -0,0 +1 @@
+validIncludeTypoScriptA = validIncludeTypoScriptA

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/Imports/validIncludeTypoScriptB.typoscript

@@ -0,0 +1 @@
+validIncludeTypoScriptC = validIncludeTypoScriptC

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/RootTemplate.csv

@@ -0,0 +1,6 @@
+"pages"
+,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","title","slug"
+,1,0,256,0,0,0,0,0,0,"FunctionalTest","/"
+"sys_template"
+,"uid","pid","sorting","deleted","hidden","starttime","endtime","t3_origuid","root","clear","include_static_file","constants","config","basedOn","includeStaticAfterBasedOn","static_file_mode"
+,1,1,256,0,0,0,0,0,1,3,"","@import 'EXT:core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/includes.typoscript","","",0,0

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Fixtures/IncludeTreeSyntaxScannerVisitor/includes.typoscript

@@ -0,0 +1,16 @@
+this.is.invalid <
+@import './Imports/validImportA.typoscript'
+@import './invalidImport.typoscript'
+@import './Imports/validImportB*.typoscript'
+@import './invalidFullDirImport/'
+<INCLUDE_TYPOSCRIPT: source="FILE:./Imports/validIncludeTypoScriptA.typoscript">
+<INCLUDE_TYPOSCRIPT: source="FILE:./invalidIncludeTypoScript.typoscript">
+[frontend.user.isLoggedIn]
+  @import './Imports/validImportC.typoscript'
+  @import './invalidImport.typoscript'
+  @import './Imports/validImportD*.typoscript'
+  <INCLUDE_TYPOSCRIPT: source="FILE:./Imports/validIncludeTypoScriptB.typoscript">
+  <INCLUDE_TYPOSCRIPT: source="FILE:./invalidIncludeTypoScript.typoscript">
+  this.is.invalid <
+[global]
+this.is.invalid <

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/Visitor/IncludeTreeSyntaxScannerVisitorTest.php

@@ -19,13 +19,26 @@
 
 use TYPO3\CMS\Core\TypoScript\IncludeTree\IncludeNode\FileInclude;
 use TYPO3\CMS\Core\TypoScript\IncludeTree\IncludeNode\IncludeInterface;
+use TYPO3\CMS\Core\TypoScript\IncludeTree\SysTemplateRepository;
+use TYPO3\CMS\Core\TypoScript\IncludeTree\SysTemplateTreeBuilder;
+use TYPO3\CMS\Core\TypoScript\IncludeTree\Traverser\IncludeTreeTraverser;
 use TYPO3\CMS\Core\TypoScript\IncludeTree\Visitor\IncludeTreeSyntaxScannerVisitor;
 use TYPO3\CMS\Core\TypoScript\Tokenizer\LosslessTokenizer;
 use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
 
 final class IncludeTreeSyntaxScannerVisitorTest extends FunctionalTestCase
 {
-    protected bool $initializeDatabase = false;
+    /**
+     * Helper method to remove line. This is more convenient
+     * to compare, we simply rely on lineNumber.
+     */
+    private function removeLineFromErrors(array $errors): array
+    {
+        foreach ($errors as &$error) {
+            unset($error['line']);
+        }
+        return $errors;
+    }
 
     public static function visitDataProvider(): iterable
     {
@@ -125,14 +138,26 @@ public function visit(IncludeInterface $node, array $expectedErrors): void
     }
 
     /**
-     * Helper method to remove line. This is more convenient
-     * to compare, we simply rely on lineNumber.
+     * @test
      */
-    private function removeLineFromErrors(array $errors): array
+    public function visitFindsEmptyImports()
     {
-        foreach ($errors as &$error) {
-            unset($error['line']);
-        }
-        return $errors;
+        $this->importCSVDataSet(__DIR__ . '/../Fixtures/IncludeTreeSyntaxScannerVisitor/RootTemplate.csv');
+        $rootline = [
+            [
+                'uid' => 1,
+                'pid' => 0,
+                'is_siteroot' => 0,
+            ],
+        ];
+        $sysTemplateRepository = $this->get(SysTemplateRepository::class);
+        $subject = $this->get(SysTemplateTreeBuilder::class);
+        $includeTree = $subject->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LosslessTokenizer());
+        $traverser = new IncludeTreeTraverser();
+        $visitor = new IncludeTreeSyntaxScannerVisitor();
+        $traverser->traverse($includeTree, [$visitor]);
+        $erroneousLineNumbers = array_column($visitor->getErrors(), 'lineNumber');
+        $expectedLineNumbers = [0, 2, 4, 6, 9, 12, 13, 15];
+        self::assertSame($expectedLineNumbers, $erroneousLineNumbers);
     }
 }

typo3/sysext/tstemplate/Resources/Private/Language/locallang_analyzer.xlf

@@ -60,6 +60,9 @@
 			<trans-unit id="syntaxError.type.brace.missing" resname="syntaxError.type.brace.missing">
 				<source>Brace missing in "%1$s", line number "%2$s"</source>
 			</trans-unit>
+			<trans-unit id="syntaxError.type.import.empty" resname="syntaxError.type.import.empty">
+				<source>Import does not find a file in "%1$s", line number "%2$s"</source>
+			</trans-unit>
 			<trans-unit id="tree.child.btn.sourceCode" resname="tree.child.btn.sourceCode">
 				<source>Show code</source>
 			</trans-unit>