feat: Display selector error locations using sourcemaps.

The source location displayed in error messages by default now will be
the source-mapped location for all selector errors.

In the CLI the generated source location as well as the source mapped
location are displayed. This is because, in my testing, the generated
source displays the error better and more reliably, but you still need a
pointer to the place in the authored source file to make the change,
especially in the case of block files that are composed of content from
multiple source files. Unfortunately, the sourcemap sometimes returns
a location that is not what the author would expect and not helpful.

Author: chriseppstein

packages/@css-blocks/cli/src/extract-lines-from-source.ts

@@ -8,12 +8,17 @@ export interface ExtractionResult {
   };
 }
 export function extractLinesFromSource(
-  range: Required<SourceRange>,
+  range: Required<SourceRange> & { source?: string },
   additionalLinesBefore = 1,
   additionalLinesAfter = 0,
-): ExtractionResult {
-  let { filename, start, end } = range;
-  let contents = fs.readFileSync(filename, "utf-8");
+): ExtractionResult | undefined {
+  let contents: string | undefined;
+  let { filename, start, end, source } = range;
+  try {
+    contents = source || fs.readFileSync(filename, "utf-8");
+  } catch (e) {
+    return;
+  }
   let allLines = contents.split(/\r?\n/);
   if (start.line <= additionalLinesBefore) {
     additionalLinesBefore = start.line - 1;

packages/@css-blocks/cli/src/index.ts

@@ -1,4 +1,13 @@
-import { BlockFactory, CssBlockError, ErrorWithPosition, Importer, NodeJsImporter, Preprocessors, hasErrorPosition } from "@css-blocks/core";
+import {
+  BlockFactory,
+  CssBlockError,
+  ErrorWithPosition,
+  Importer,
+  NodeJsImporter,
+  Preprocessors,
+  hasErrorPosition,
+  hasMappedPosition,
+} from "@css-blocks/core";
 import chalk = require("chalk");
 import fse = require("fs-extra");
 import path = require("path");
@@ -162,32 +171,40 @@ export class CLI {
     if (!hasErrorPosition(loc)) {
       return;
     }
-    loc.end.line = 4;
     let filename = path.relative(process.cwd(), path.resolve(loc && loc.filename || blockFileRelative));
-    let context: ExtractionResult | undefined;
-    let lineNumber: number | undefined;
-    context = extractLinesFromSource(loc, 1, 1);
-    lineNumber = loc.start.line - context.additionalLines.before;
-    if (context) {
+    this.println("\t" + this.chalk.bold.redBright(e.origMessage));
+    if (hasMappedPosition(loc)) {
       this.println(
-        this.chalk.bold.white("\tAt"),
-        this.chalk.bold.whiteBright(`${filename}:${loc.start.line}:${loc.start.column}`),
-        `${e.origMessage}`,
+        this.chalk.bold.white("\tAt compiled output of"),
+        this.chalk.bold.whiteBright(`${loc.generated.filename}:${loc.generated.start.line}:${loc.generated.start.column}`),
       );
-      for (let i = 0; i < context.lines.length; i++) {
-        let prefix;
-        let line = context.lines[i];
-        if (i < context.additionalLines.before ||
-            i >= context.lines.length - context.additionalLines.after) {
-          prefix = this.chalk.bold(`${lineNumber}: `);
-        } else {
-          prefix = this.chalk.bold.redBright(`${lineNumber}: `);
-          let {before, during, after } = this.splitLineOnErrorRange(line, lineNumber, loc);
-          line = `${before}${this.chalk.underline.redBright(during)}${after}`;
-        }
-        this.println("\t" + prefix + line);
-        if (lineNumber) lineNumber++;
+      this.displaySnippet(extractLinesFromSource(loc.generated, 1, 1), loc.generated);
+    }
+    this.println(
+      this.chalk.bold.white(hasMappedPosition(loc) ? "\tSource Mapped to" : "\tAt"),
+      this.chalk.bold.whiteBright(`${filename}:${loc.start.line}:${loc.start.column}`),
+    );
+    this.displaySnippet(extractLinesFromSource(loc, 1, 1), loc);
+  }
+
+  displaySnippet(context: ExtractionResult | undefined, loc: ErrorWithPosition) {
+    if (!context) return;
+    let lineNumber: number | undefined;
+    lineNumber = loc.start.line - context.additionalLines.before;
+
+    for (let i = 0; i < context.lines.length; i++) {
+      let prefix;
+      let line = context.lines[i];
+      if (i < context.additionalLines.before ||
+          i >= context.lines.length - context.additionalLines.after) {
+        prefix = this.chalk.bold(`${lineNumber}:${line ? " " : ""}`);
+      } else {
+        prefix = this.chalk.bold.redBright(`${lineNumber}:${line ? " " : ""}`);
+        let {before, during, after } = this.splitLineOnErrorRange(line, lineNumber, loc);
+        line = `${before}${this.chalk.underline.redBright(during)}${after}`;
       }
+      this.println("\t" + prefix + line);
+      if (lineNumber) lineNumber++;
     }
   }
 

packages/@css-blocks/cli/test/cli-test.ts

@@ -31,10 +31,10 @@ describe("validate", () => {
     await cli.run(["validate", fixture("basic/error.block.css")]);
     assert.equal(cli.output,
                  `error\t${relFixture("basic/error.block.css")}
-\tAt ${relFixture("basic/error.block.css")}:1:5 Two distinct classes cannot be selected on the same element: .foo.bar
+\tTwo distinct classes cannot be selected on the same element: .foo.bar
+\tAt ${relFixture("basic/error.block.css")}:1:5
 \t1: .foo.bar {
 \t2:   color: red;
-\t3: }
 Found 1 error in 1 file.
 `);
     assert.equal(cli.exitCode, 1);
@@ -44,10 +44,10 @@ Found 1 error in 1 file.
     await cli.run(["validate", fixture("basic/transitive-error.block.css")]);
     assert.equal(cli.output,
                  `error\t${relFixture("basic/transitive-error.block.css")}
-\tAt ${relFixture("basic/error.block.css")}:1:5 Two distinct classes cannot be selected on the same element: .foo.bar
+\tTwo distinct classes cannot be selected on the same element: .foo.bar
+\tAt ${relFixture("basic/error.block.css")}:1:5
 \t1: .foo.bar {
 \t2:   color: red;
-\t3: }
 Found 1 error in 1 file.
 `);
     assert.equal(cli.exitCode, 1);
@@ -78,7 +78,18 @@ describe("validate with preprocessors", () => {
   it("can check syntax for a bad block file", async () => {
     let cli = new CLI();
     await cli.run(["validate", "--preprocessors", distFile("test/preprocessors"), fixture("scss/error.block.scss")]);
-    assert.equal(cli.output.split(/\n/)[1].trim(), `At ${relFixture("scss/error.block.scss")}:5:5 Two distinct classes cannot be selected on the same element: .foo.bar`);
+    assert.equal(cli.output, `error	test/fixtures/scss/error.block.scss
+\tTwo distinct classes cannot be selected on the same element: .foo.bar
+\tAt compiled output of test/fixtures/scss/error.block.scss:5:5
+\t4:
+\t5: .foo.bar {
+\t6:   color: blue;
+\tSource Mapped to test/fixtures/scss/error.block.scss:3:5
+\t2:
+\t3: .foo {
+\t4:   color: red;
+Found 1 error in 1 file.
+`);
     assert.equal(cli.exitCode, 1);
   });
 });

packages/@css-blocks/cli/test/fixtures/basic/attribute-error.block.css

@@ -0,0 +1,3 @@
+.foo[disabled=disabled] {
+  color: red;
+}

packages/@css-blocks/cli/test/fixtures/scss/another-error.block.scss

@@ -0,0 +1,7 @@
+@import "mixins";
+
+.a {
+  @include plus-b() {
+    color: purple;
+  }
+}

packages/@css-blocks/cli/test/fixtures/scss/error.block.scss

@@ -1,6 +1,8 @@
+@import "mixins";
+
 .foo {
   color: red;
-  &.bar {
+  @include add-bar() {
     color: blue;
   }
-}
+}

packages/@css-blocks/cli/test/fixtures/scss/mixins.scss

@@ -0,0 +1,24 @@
+@mixin add-bar() {
+  &.bar {
+    @content;
+  }
+}
+
+@mixin plus-b() {
+  & + .bbb {
+    @content;
+  }
+}
+
+@mixin in-state($state-name, $state-value: null) {
+  @if $state-value {
+    &[state|#{$state-name}=#{$state-value}] {
+      @content;
+    }
+  }
+  @else {
+    &[state|#{$state-name}] {
+      @content;
+    }
+  }
+}

packages/@css-blocks/cli/test/fixtures/scss/tagname-error.block.scss

@@ -0,0 +1,7 @@
+@import "mixins";
+
+div         {
+  @include in-state(selected) {
+    outline: 1px dotted black;
+  }
+}

packages/@css-blocks/cli/test/preprocessors.ts

@@ -9,6 +9,7 @@ const scss: Preprocessors["scss"] = async (fullPath, content, _configuration, _s
         outFile: fullPath.replace("scss", "css"),
         data: content,
         sourceMap: true,
+        sourceMapContents: true,
         outputStyle: "expanded",
         indentedSyntax: false,
       },

packages/@css-blocks/core/src/BlockParser/BlockFactory.ts

@@ -175,7 +175,7 @@ export class BlockFactory {
     let sourceMap = sourceMapFromProcessedFile(preprocessResult);
     let content = preprocessResult.content;
     if (sourceMap) {
-      content = annotateCssContentWithSourceMap(content, sourceMap);
+      content = annotateCssContentWithSourceMap(this.configuration, filename, content, sourceMap);
     }
     let root = await this.postcssImpl.parse(content, { from: filename });
 

packages/@css-blocks/core/src/BlockParser/BlockParser.ts

@@ -85,10 +85,10 @@ export class BlockParser {
     await globalAttributes(root, block, sourceFile);
     // Parse all block styles and build block tree.
     debug(` - Construct Block`);
-    await constructBlock(root, block, debugIdent);
+    await constructBlock(root, block, this.factory.configuration, debugIdent);
     // Verify that external blocks referenced have been imported, have defined the attribute being selected, and have marked it as a global state.
     debug(` - Assert Foreign Globals`);
-    await assertForeignGlobalAttribute(root, block, debugIdent);
+    await assertForeignGlobalAttribute(this.factory.configuration, root, block, debugIdent);
     // Construct block extensions and validate.
     debug(` - Extend Block`);
     await extendBlock(root, block, debugIdent);

packages/@css-blocks/core/src/BlockParser/features/assert-foreign-global-attribute.ts

@@ -1,6 +1,7 @@
 import { postcss, postcssSelectorParser as selectorParser } from "opticss";
 
 import { Block } from "../../BlockTree";
+import { Configuration } from "../../configuration";
 import * as errors from "../../errors";
 import { selectorSourceRange as loc } from "../../SourceLocation";
 import { isAttributeNode, toAttrToken } from "../block-intermediates";
@@ -12,7 +13,7 @@ import { isAttributeNode, toAttrToken } from "../block-intermediates";
  * @param rule The rule referencing the external block.
  * @param obj The parsed node making the external reference.
  */
-export async function assertForeignGlobalAttribute(root: postcss.Root, block: Block, file: string) {
+export async function assertForeignGlobalAttribute(configuration: Configuration, root: postcss.Root, block: Block, file: string) {
 
   root.walkRules((rule) => {
 
@@ -35,30 +36,30 @@ export async function assertForeignGlobalAttribute(root: postcss.Root, block: Bl
           if (!isAttributeNode(node)) {
             throw new errors.InvalidBlockSyntax(
               `Only global states from other blocks can be used in selectors: ${rule.selector}`,
-              loc(file, rule, node));
+              loc(configuration, block.stylesheet, file, rule, node));
           }
 
           // If referenced block does not exist, throw.
           let otherBlock = block.getReferencedBlock(blockName.value);
           if (!otherBlock) {
             throw new errors.InvalidBlockSyntax(
               `No Block named "${blockName.value}" found in scope: ${rule.selector}`,
-              loc(file, rule, node));
+              loc(configuration, block.stylesheet, file, rule, node));
           }
 
           // If state referenced does not exist on external block, throw
           let otherAttr = otherBlock.rootClass.getAttributeValue(toAttrToken(node));
           if (!otherAttr) {
             throw new errors.InvalidBlockSyntax(
               `No state ${node.toString()} found in : ${rule.selector}`,
-              loc(file, rule, node));
+              loc(configuration, block.stylesheet, file, rule, node));
           }
 
           // If external state is not set as global, throw.
           if (!otherAttr.isGlobal) {
             throw new errors.InvalidBlockSyntax(
               `${node.toString()} is not global: ${rule.selector}`,
-              loc(file, rule, node));
+              loc(configuration, block.stylesheet, file, rule, node));
           }
 
         }