Feature: check empty string reference for other resource in HTML(and …

…SVG) (#7318)
parcel-bundler · Nov 28, 2021 · 5b606ea · 5b606ea
1 parent be65b92
commit 5b606ea
Show file tree

Hide file tree

Showing 8 changed files with 165 additions and 3 deletions.
diff --git a/packages/core/integration-tests/test/html.js b/packages/core/integration-tests/test/html.js
@@ -2704,4 +2704,98 @@ describe('html', function () {
     assert(output.includes('<filter'));
     assert(output.includes('<feGaussianBlur in="SourceGraphic" stdDeviation='));
   });
+
+  it('should throw error with empty string reference to other resource', async function () {
+    await assert.rejects(
+      () =>
+        bundle(
+          path.join(__dirname, 'integration/html-empty-reference/index.html'),
+          {
+            mode: 'production',
+          },
+        ),
+      {
+        name: 'BuildError',
+        diagnostics: [
+          {
+            message: 'src should not be empty string',
+            origin: '@parcel/transformer-html',
+            codeFrames: [
+              {
+                filePath: path.join(
+                  __dirname,
+                  'integration/html-empty-reference/index.html',
+                ),
+                language: 'html',
+                codeHighlights: [
+                  {
+                    start: {
+                      column: 1,
+                      line: 1,
+                    },
+                    end: {
+                      column: 14,
+                      line: 1,
+                    },
+                  },
+                ],
+              },
+            ],
+          },
+
+          {
+            message: 'src should not be empty string',
+            origin: '@parcel/transformer-html',
+            codeFrames: [
+              {
+                filePath: path.join(
+                  __dirname,
+                  'integration/html-empty-reference/index.html',
+                ),
+                language: 'html',
+                codeHighlights: [
+                  {
+                    start: {
+                      column: 1,
+                      line: 2,
+                    },
+                    end: {
+                      column: 24,
+                      line: 2,
+                    },
+                  },
+                ],
+              },
+            ],
+          },
+
+          {
+            message: 'href should not be empty string',
+            origin: '@parcel/transformer-html',
+            codeFrames: [
+              {
+                filePath: path.join(
+                  __dirname,
+                  'integration/html-empty-reference/index.html',
+                ),
+                language: 'html',
+                codeHighlights: [
+                  {
+                    start: {
+                      column: 1,
+                      line: 3,
+                    },
+                    end: {
+                      column: 16,
+                      line: 3,
+                    },
+                  },
+                ],
+              },
+            ],
+          },
+        ],
+      },
+    );
+  });
 });
diff --git a/packages/core/integration-tests/test/integration/html-empty-reference/index.html b/packages/core/integration-tests/test/integration/html-empty-reference/index.html
@@ -0,0 +1,3 @@
+<img src="" />
+<script src=""></script>
+<link href="" />
diff --git a/packages/transformers/html/package.json b/packages/transformers/html/package.json
@@ -22,6 +22,7 @@
   "dependencies": {
     "@parcel/hash": "^2.0.1",
     "@parcel/plugin": "^2.0.1",
+    "@parcel/diagnostic": "^2.0.1",
     "nullthrows": "^1.1.1",
     "posthtml": "^0.16.5",
     "posthtml-parser": "^0.10.1",

diff --git a/packages/transformers/html/src/HTMLTransformer.js b/packages/transformers/html/src/HTMLTransformer.js
@@ -10,6 +10,7 @@ import {render} from 'posthtml-render';
 import semver from 'semver';
 import collectDependencies from './dependencies';
 import extractInlineAssets from './inline';
+import ThrowableDiagnostic from '@parcel/diagnostic';
 
 export default (new Transformer({
   canReuseAST({ast}) {
@@ -33,9 +34,27 @@ export default (new Transformer({
     if (asset.type === 'htm') {
       asset.type = 'html';
     }
+
     asset.bundleBehavior = 'isolated';
     let ast = nullthrows(await asset.getAST());
-    let hasScripts = collectDependencies(asset, ast);
+    let hasScripts;
+    try {
+      hasScripts = collectDependencies(asset, ast);
+    } catch (errors) {
+      throw new ThrowableDiagnostic({
+        diagnostic: errors.map(error => ({
+          message: error.message,
+          origin: '@parcel/transformer-html',
+          codeFrames: [
+            {
+              filePath: error.filePath,
+              language: 'html',
+              codeHighlights: [error.loc],
+            },
+          ],
+        })),
+      });
+    }
 
     const {assets: inlineAssets, hasScripts: hasInlineScripts} =
       extractInlineAssets(asset, ast);

diff --git a/packages/transformers/html/src/dependencies.js b/packages/transformers/html/src/dependencies.js
@@ -3,7 +3,6 @@
 import type {AST, MutableAsset} from '@parcel/types';
 import type {PostHTMLNode} from 'posthtml';
 import PostHTML from 'posthtml';
-
 // A list of all attributes that may produce a dependency
 // Based on https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes
 const ATTRS = {
@@ -115,6 +114,7 @@ export default function collectDependencies(
   let isDirty = false;
   let hasScripts = false;
   let seen = new Set();
+  const errors = [];
   PostHTML().walk.call(ast.program, node => {
     let {tag, attrs} = node;
     if (!attrs || seen.has(node)) {
@@ -254,6 +254,15 @@ export default function collectDependencies(
         continue;
       }
 
+      // Check for empty string
+      if (attrs[attr].length === 0) {
+        errors.push({
+          message: `${attr} should not be empty string`,
+          filePath: asset.filePath,
+          loc: node.location,
+        });
+      }
+
       let elements = ATTRS[attr];
       if (elements && elements.includes(node.tag)) {
         let depHandler = getAttrDepHandler(attr);
@@ -274,5 +283,9 @@ export default function collectDependencies(
     return node;
   });
 
+  if (errors.length > 0) {
+    throw errors;
+  }
+
   return hasScripts;
 }
diff --git a/packages/transformers/svg/package.json b/packages/transformers/svg/package.json
@@ -22,6 +22,7 @@
   "dependencies": {
     "@parcel/hash": "^2.0.1",
     "@parcel/plugin": "^2.0.1",
+    "@parcel/diagnostic": "^2.0.1",
     "nullthrows": "^1.1.1",
     "posthtml": "^0.16.5",
     "posthtml-parser": "^0.10.1",

diff --git a/packages/transformers/svg/src/SVGTransformer.js b/packages/transformers/svg/src/SVGTransformer.js
@@ -7,6 +7,7 @@ import {parser as parse} from 'posthtml-parser';
 import {render} from 'posthtml-render';
 import collectDependencies from './dependencies';
 import extractInlineAssets from './inline';
+import ThrowableDiagnostic from '@parcel/diagnostic';
 
 export default (new Transformer({
   canReuseAST({ast}) {
@@ -36,7 +37,23 @@ export default (new Transformer({
 
     const ast = nullthrows(await asset.getAST());
 
-    collectDependencies(asset, ast);
+    try {
+      collectDependencies(asset, ast);
+    } catch (errors) {
+      throw new ThrowableDiagnostic({
+        diagnostic: errors.map(error => ({
+          message: error.message,
+          origin: '@parcel/transformer-svg',
+          codeFrames: [
+            {
+              filePath: error.filePath,
+              language: 'svg',
+              codeHighlights: [error.loc],
+            },
+          ],
+        })),
+      });
+    }
 
     const inlineAssets = extractInlineAssets(asset, ast);
 

diff --git a/packages/transformers/svg/src/dependencies.js b/packages/transformers/svg/src/dependencies.js
@@ -77,6 +77,7 @@ const OPTIONS = {
 
 export default function collectDependencies(asset: MutableAsset, ast: AST) {
   let isDirty = false;
+  const errors = [];
   PostHTML().walk.call(ast.program, node => {
     // Ideally we'd have location information for specific attributes...
     let getLoc = () =>
@@ -106,6 +107,15 @@ export default function collectDependencies(asset: MutableAsset, ast: AST) {
         continue;
       }
 
+      // Check for empty string
+      if (attrs[attr].length === 0) {
+        errors.push({
+          message: `${attr} should not be empty string`,
+          filePath: asset.filePath,
+          loc: node.location,
+        });
+      }
+
       const elements = ATTRS[attr];
       if (elements && elements.includes(node.tag)) {
         let options = OPTIONS[tag]?.[attr];
@@ -143,6 +153,10 @@ export default function collectDependencies(asset: MutableAsset, ast: AST) {
     return node;
   });
 
+  if (errors.length > 0) {
+    throw errors;
+  }
+
   if (isDirty) {
     asset.setAST(ast);
   }