Skip to content

Commit f7f2dfb

Browse files
committed
feat: Track ranges instead of only the start position for errors.
1 parent 311d380 commit f7f2dfb

22 files changed

+167
-114
lines changed

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { BlockFactory, CssBlockError, Importer, NodeJsImporter, Preprocessors } from "@css-blocks/core";
1+
import { BlockFactory, CssBlockError, Importer, NodeJsImporter, Preprocessors, hasErrorPosition } from "@css-blocks/core";
22
import chalk = require("chalk");
33
import fse = require("fs-extra");
44
import path = require("path");
@@ -137,8 +137,8 @@ export class CLI {
137137
let loc = e.location;
138138
let filename = path.relative(process.cwd(), path.resolve(loc && loc.filename || blockFile));
139139
let message = `${this.chalk.red("error")}\t${this.chalk.whiteBright(filename)}`;
140-
if (loc && loc.filename && loc.line && loc.column) {
141-
message += `:${loc.line}:${loc.column}`;
140+
if (hasErrorPosition(loc)) {
141+
message += `:${loc.start.line}:${loc.start.column}`;
142142
}
143143
message += ` ${e.origMessage}`;
144144
this.println(message);

packages/@css-blocks/core/src/Analyzer/validations/index.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,15 @@ export class TemplateValidator {
9999
validate(templateAnalysis: Analysis<keyof TemplateTypes>, element: ElementAnalysis<any, any, any>) {
100100

101101
function err (message: string, locInfo?: errors.ErrorLocation | undefined | null, details?: string) {
102+
let location = locInfo || {
103+
filename: templateAnalysis.template.identifier,
104+
start: element.sourceLocation.start,
105+
end: element.sourceLocation.end
106+
|| { line: element.sourceLocation.start.line,
107+
column: (element.sourceLocation.start.column || 0) + 1},
108+
};
102109
throw new errors.TemplateAnalysisError(
103-
message, locInfo || element.sourceLocation.start, details);
110+
message, location, details);
104111
}
105112

106113
this.validators.forEach((func) => {

packages/@css-blocks/core/src/BlockCompiler/ConflictResolver.ts

+9-9
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { Block, BlockClass, Style } from "../BlockTree";
77
import { ResolvedConfiguration } from "../configuration";
88
import * as errors from "../errors";
99
import { QueryKeySelector } from "../query";
10-
import { SourceLocation, sourceLocation } from "../SourceLocation";
10+
import { SourceFile, SourceRange, sourceRange } from "../SourceLocation";
1111
import { expandProp } from "../util/propertyParser";
1212

1313
import { Conflicts, detectConflicts } from "./conflictDetection";
@@ -179,7 +179,7 @@ export class ConflictResolver {
179179

180180
// Throw if resolutions are not all before or after values for the same property.
181181
if (localDecls.length && foundRes) {
182-
throw new errors.InvalidBlockSyntax(`Resolving ${decl.prop} must happen either before or after all other values for ${decl.prop}.`, this.sourceLocation(block, decl));
182+
throw new errors.InvalidBlockSyntax(`Resolving ${decl.prop} must happen either before or after all other values for ${decl.prop}.`, this.sourceRange(block, decl));
183183
}
184184

185185
// Save the applicable local decl.
@@ -188,23 +188,23 @@ export class ConflictResolver {
188188

189189
// If no local declarations found setting this value, throw.
190190
if (!localDecls.length) {
191-
throw new errors.InvalidBlockSyntax(`Cannot resolve ${decl.prop} without a concrete value.`, this.sourceLocation(block, decl));
191+
throw new errors.InvalidBlockSyntax(`Cannot resolve ${decl.prop} without a concrete value.`, this.sourceRange(block, decl));
192192
}
193193

194194
// Look up the block that contains the requested resolution.
195195
let other: Style | undefined = block.lookup(resolution.path);
196196
if (!other) {
197-
throw new errors.InvalidBlockSyntax(`Cannot find ${resolution.path}`, decl.source && decl.source.start);
197+
throw new errors.InvalidBlockSyntax(`Cannot find ${resolution.path}`, this.sourceRange(block, decl));
198198
}
199199

200200
// If trying to resolve rule from the same block, throw.
201201
if (block.equal(other && other.block)) {
202-
throw new errors.InvalidBlockSyntax(`Cannot resolve conflicts with your own block.`, this.sourceLocation(block, decl));
202+
throw new errors.InvalidBlockSyntax(`Cannot resolve conflicts with your own block.`, this.sourceRange(block, decl));
203203
}
204204

205205
// If trying to resolve (read: not inheritance resolution) from an ancestor block, throw.
206206
else if (!resolution.isInherited && other && other.block.isAncestorOf(block)) {
207-
throw new errors.InvalidBlockSyntax(`Cannot resolve conflicts with ancestors of your own block.`, this.sourceLocation(block, decl));
207+
throw new errors.InvalidBlockSyntax(`Cannot resolve conflicts with ancestors of your own block.`, this.sourceRange(block, decl));
208208
}
209209

210210
// Crawl up inheritance tree of the other block and attempt to resolve the conflict at each level.
@@ -217,7 +217,7 @@ export class ConflictResolver {
217217
// If no conflicting Declarations were found (aka: calling for a resolution
218218
// with nothing to resolve), throw error.
219219
if (!resolution.isInherited && foundConflict === ConflictType.noConflict) {
220-
throw new errors.InvalidBlockSyntax(`There are no conflicting values for ${decl.prop} found in any selectors targeting ${resolution.path}.`, this.sourceLocation(block, decl));
220+
throw new errors.InvalidBlockSyntax(`There are no conflicting values for ${decl.prop} found in any selectors targeting ${resolution.path}.`, this.sourceRange(block, decl));
221221
}
222222

223223
// Remove resolution Declaration. Do after traversal because otherwise we mess up postcss' iterator.
@@ -433,8 +433,8 @@ export class ConflictResolver {
433433
// Wrap our list of CompoundSelectors in ParsedSelector containers and return.
434434
return mergedSelectors.map(sel => new ParsedSelector(sel, sel.toString()));
435435
}
436-
sourceLocation(block: Block, node: postcss.Node): SourceLocation | undefined {
436+
sourceRange(block: Block, node: postcss.Node): SourceRange | SourceFile | undefined {
437437
let blockPath = this.config.importer.debugIdentifier(block.identifier, this.config);
438-
return sourceLocation(blockPath, node);
438+
return sourceRange(blockPath, node);
439439
}
440440
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { postcss, postcssSelectorParser as selectorParser } from "opticss";
22

33
import { Block } from "../../BlockTree";
44
import * as errors from "../../errors";
5-
import { selectorSourceLocation as loc } from "../../SourceLocation";
5+
import { selectorSourceRange as loc } from "../../SourceLocation";
66
import { isAttributeNode, toAttrToken } from "../block-intermediates";
77

88
/**

packages/@css-blocks/core/src/BlockParser/features/composes-block.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { isRule } from "opticss/dist/src/util/cssIntrospection";
44
import { COMPOSES } from "../../BlockSyntax";
55
import { Block } from "../../BlockTree";
66
import * as errors from "../../errors";
7-
import { sourceLocation } from "../../SourceLocation";
7+
import { sourceRange } from "../../SourceLocation";
88
import { getStyleTargets } from "../block-intermediates";
99
import { stripQuotes } from "../utils";
1010

@@ -17,24 +17,24 @@ import { stripQuotes } from "../utils";
1717
*/
1818
export async function composeBlock(root: postcss.Root, block: Block, sourceFile: string) {
1919
root.walkDecls(COMPOSES, (decl) => {
20-
if (!isRule(decl.parent)) { throw new errors.InvalidBlockSyntax(`The "composes" property may only be used in a rule set.`, sourceLocation(sourceFile, decl)); }
20+
if (!isRule(decl.parent)) { throw new errors.InvalidBlockSyntax(`The "composes" property may only be used in a rule set.`, sourceRange(sourceFile, decl)); }
2121
let rule = decl.parent;
2222

2323
// TODO: Move to Block Syntax as parseBlockRefList().
2424
let refNames = decl.value.split(/,\s*/).map(stripQuotes);
2525
for (let refName of refNames) {
2626
let refStyle = block.lookup(refName);
2727
if (!refStyle) {
28-
throw new errors.InvalidBlockSyntax(`No style "${refName}" found.`, sourceLocation(sourceFile, decl));
28+
throw new errors.InvalidBlockSyntax(`No style "${refName}" found.`, sourceRange(sourceFile, decl));
2929
}
3030
if (refStyle.block === block) {
31-
throw new errors.InvalidBlockSyntax(`Styles from the same Block may not be composed together.`, sourceLocation(sourceFile, decl));
31+
throw new errors.InvalidBlockSyntax(`Styles from the same Block may not be composed together.`, sourceRange(sourceFile, decl));
3232
}
3333

3434
const parsedSel = block.getParsedSelectors(rule);
3535
for (let sel of parsedSel) {
3636
if (sel.selector.next) {
37-
throw new errors.InvalidBlockSyntax(`Style composition is not allowed in rule sets with a scope selector.`, sourceLocation(sourceFile, decl));
37+
throw new errors.InvalidBlockSyntax(`Style composition is not allowed in rule sets with a scope selector.`, sourceRange(sourceFile, decl));
3838
}
3939
let foundStyles = getStyleTargets(block, sel.selector);
4040
for (let blockClass of foundStyles.blockClasses) {

packages/@css-blocks/core/src/BlockParser/features/construct-block.ts

+24-24
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { CompoundSelector, ParsedSelector, postcss, postcssSelectorParser as sel
22

33
import { Block, Style } from "../../BlockTree";
44
import * as errors from "../../errors";
5-
import { selectorSourceLocation as loc, sourceLocation } from "../../SourceLocation";
5+
import { selectorSourceRange as range, sourceRange } from "../../SourceLocation";
66
import {
77
BlockType,
88
NodeAndType,
@@ -40,7 +40,7 @@ function shouldBeParsedAsBlockSelector(rule: postcss.Rule): boolean {
4040
function getParsedSelectors(block: Block, rule: postcss.Rule, file: string): ParsedSelector[] {
4141
let res;
4242
try { res = block.getParsedSelectors(rule); }
43-
catch (e) { throw new errors.InvalidBlockSyntax(e.message, sourceLocation(file, rule)); }
43+
catch (e) { throw new errors.InvalidBlockSyntax(e.message, sourceRange(file, rule)); }
4444
return res;
4545
}
4646

@@ -118,7 +118,7 @@ function assertValidSelector(block: Block, rule: postcss.Rule, selector: ParsedS
118118
if (keyObject.blockName) {
119119
throw new errors.InvalidBlockSyntax(
120120
`Cannot style values from other blocks: ${rule.selector}`,
121-
loc(file, rule, keyObject.node));
121+
range(file, rule, keyObject.node));
122122
}
123123

124124
// Fetch and validate our first `CompoundSelector`
@@ -148,23 +148,23 @@ function assertValidSelector(block: Block, rule: postcss.Rule, selector: ParsedS
148148
if (!LEGAL_COMBINATORS.has(combinator)) {
149149
throw new errors.InvalidBlockSyntax(
150150
`Illegal Combinator '${combinator}': ${rule.selector}`,
151-
loc(file, rule, currentCompoundSel.next.combinator),
151+
range(file, rule, currentCompoundSel.next.combinator),
152152
);
153153
}
154154

155155
// Class level objects cannot be ancestors of root level objects
156156
if (isClassLevelObject(currentObject) && nextLevelIsRoot && SIBLING_COMBINATORS.has(combinator)) {
157157
throw new errors.InvalidBlockSyntax(
158158
`A class is never a sibling of a ${blockTypeName(nextObject.blockType)}: ${rule.selector}`,
159-
loc(file, rule, selector.selector.nodes[0]),
159+
range(file, rule, selector.selector.nodes[0]),
160160
);
161161
}
162162

163163
// Once you go to the class level there's no combinator that gets you back to the root level
164164
if (foundClassLevel && nextLevelIsRoot) {
165165
throw new errors.InvalidBlockSyntax(
166166
`Illegal scoping of a ${blockTypeName(currentObject.blockType)}: ${rule.selector}`,
167-
loc(file, rule, currentCompoundSel.next.combinator),
167+
range(file, rule, currentCompoundSel.next.combinator),
168168
);
169169
}
170170

@@ -174,7 +174,7 @@ function assertValidSelector(block: Block, rule: postcss.Rule, selector: ParsedS
174174
if (!foundObjects.every(f => f.node.toString() === nextObject.node.toString())) {
175175
throw new errors.InvalidBlockSyntax(
176176
`Illegal scoping of a ${blockTypeName(currentObject.blockType)}: ${rule.selector}`,
177-
loc(file, rule, currentCompoundSel.next.combinator),
177+
range(file, rule, currentCompoundSel.next.combinator),
178178
);
179179
}
180180
}
@@ -183,7 +183,7 @@ function assertValidSelector(block: Block, rule: postcss.Rule, selector: ParsedS
183183
if (isRootLevelObject(currentObject) && nextLevelIsClass && SIBLING_COMBINATORS.has(combinator)) {
184184
throw new errors.InvalidBlockSyntax(
185185
`A ${blockTypeName(nextObject.blockType)} cannot be a sibling with a ${blockTypeName(currentObject.blockType)}: ${rule.selector}`,
186-
loc(file, rule, currentCompoundSel.next.combinator),
186+
range(file, rule, currentCompoundSel.next.combinator),
187187
);
188188
}
189189

@@ -195,12 +195,12 @@ function assertValidSelector(block: Block, rule: postcss.Rule, selector: ParsedS
195195
if (conflictObj.blockType === nextObject.blockType) {
196196
throw new errors.InvalidBlockSyntax(
197197
`Distinct ${blockTypeName(conflictObj.blockType, { plural: true })} cannot be combined: ${rule.selector}`,
198-
loc(file, rule, nextObject.node),
198+
range(file, rule, nextObject.node),
199199
);
200200
} else {
201201
throw new errors.InvalidBlockSyntax(
202202
`Cannot combine a ${blockTypeName(conflictObj.blockType)} with a ${blockTypeName(nextObject.blockType)}}: ${rule.selector}`,
203-
loc(file, rule, nextObject.node),
203+
range(file, rule, nextObject.node),
204204
);
205205
}
206206
}
@@ -233,7 +233,7 @@ function assertBlockObject(block: Block, sel: CompoundSelector, rule: postcss.Ru
233233
if (!refBlock) {
234234
throw new errors.InvalidBlockSyntax(
235235
`Tag name selectors are not allowed: ${rule.selector}`,
236-
loc(file, rule, blockName),
236+
range(file, rule, blockName),
237237
);
238238
}
239239
}
@@ -243,7 +243,7 @@ function assertBlockObject(block: Block, sel: CompoundSelector, rule: postcss.Ru
243243
if (nonStateAttribute) {
244244
throw new errors.InvalidBlockSyntax(
245245
`Cannot select attributes other than states: ${rule.selector}`,
246-
loc(file, rule, nonStateAttribute),
246+
range(file, rule, nonStateAttribute),
247247
);
248248
}
249249

@@ -254,7 +254,7 @@ function assertBlockObject(block: Block, sel: CompoundSelector, rule: postcss.Ru
254254
if (pseudo.value === ":not" || pseudo.value === ":matches") {
255255
throw new errors.InvalidBlockSyntax(
256256
`The ${pseudo.value}() pseudoclass cannot be used: ${rule.selector}`,
257-
loc(file, rule, pseudo),
257+
range(file, rule, pseudo),
258258
);
259259
}
260260
}
@@ -275,7 +275,7 @@ function assertBlockObject(block: Block, sel: CompoundSelector, rule: postcss.Ru
275275
} else {
276276
throw new errors.InvalidBlockSyntax(
277277
`External Block ${n} must be the first selector in "${rule.selector}"`,
278-
loc(file, rule, sel.nodes[0]),
278+
range(file, rule, sel.nodes[0]),
279279
);
280280
}
281281
}
@@ -292,7 +292,7 @@ function assertBlockObject(block: Block, sel: CompoundSelector, rule: postcss.Ru
292292
if (found.blockType === BlockType.class || found.blockType === BlockType.classAttribute) {
293293
throw new errors.InvalidBlockSyntax(
294294
`${n} cannot be on the same element as ${found.node}: ${rule.selector}`,
295-
loc(file, rule, sel.nodes[0]),
295+
range(file, rule, sel.nodes[0]),
296296
);
297297
}
298298
}
@@ -305,13 +305,13 @@ function assertBlockObject(block: Block, sel: CompoundSelector, rule: postcss.Ru
305305
if (n.value && n.operator !== "=") {
306306
throw new errors.InvalidBlockSyntax(
307307
`A state with a value must use the = operator (found ${n.operator} instead).`,
308-
loc(file, rule, n),
308+
range(file, rule, n),
309309
);
310310
}
311311
if (!found) {
312312
throw new errors.InvalidBlockSyntax(
313313
`States without an explicit :scope or class selector are not supported: ${rule.selector}`,
314-
loc(file, rule, n),
314+
range(file, rule, n),
315315
);
316316
} else if (found.blockType === BlockType.class || found.blockType === BlockType.classAttribute) {
317317
found = { node: n, blockType: BlockType.classAttribute };
@@ -332,17 +332,17 @@ function assertBlockObject(block: Block, sel: CompoundSelector, rule: postcss.Ru
332332
if (found.blockType === BlockType.root) {
333333
throw new errors.InvalidBlockSyntax(
334334
`${n} cannot be on the same element as ${found.node}: ${rule.selector}`,
335-
loc(file, rule, sel.nodes[0]));
335+
range(file, rule, sel.nodes[0]));
336336
} else if (found.blockType === BlockType.class) {
337337
if (n.toString() !== found.node.toString()) {
338338
throw new errors.InvalidBlockSyntax(
339339
`Two distinct classes cannot be selected on the same element: ${rule.selector}`,
340-
loc(file, rule, n));
340+
range(file, rule, n));
341341
}
342342
} else if (found.blockType === BlockType.classAttribute || found.blockType === BlockType.attribute) {
343343
throw new errors.InvalidBlockSyntax(
344344
`The class must precede the state: ${rule.selector}`,
345-
loc(file, rule, sel.nodes[0]));
345+
range(file, rule, sel.nodes[0]));
346346
}
347347
}
348348
}
@@ -355,21 +355,21 @@ function assertBlockObject(block: Block, sel: CompoundSelector, rule: postcss.Ru
355355
if (!result) {
356356
throw new errors.InvalidBlockSyntax(
357357
`Missing block object in selector component '${sel.nodes.join("")}': ${rule.selector}`,
358-
loc(file, rule, sel.nodes[0]));
358+
range(file, rule, sel.nodes[0]));
359359
}
360360

361361
if (isExternalBlock(result)) {
362362
let external = block.getReferencedBlock(result.node.value!);
363-
if (!external) { throw new errors.InvalidBlockSyntax(``, loc(file, rule, sel.nodes[0])); }
363+
if (!external) { throw new errors.InvalidBlockSyntax(``, range(file, rule, sel.nodes[0])); }
364364
let globalStates = external.rootClass.allAttributeValues().filter((a) => a.isGlobal);
365365
if (!globalStates.length) {
366366
throw new errors.InvalidBlockSyntax(
367367
`External Block '${result.node.value}' has no global states.`,
368-
loc(file, rule, sel.nodes[0]));
368+
range(file, rule, sel.nodes[0]));
369369
}
370370
throw new errors.InvalidBlockSyntax(
371371
`Missing global state selector on external Block '${result.node.value}'. Did you mean one of: ${globalStates.map((s) => s.asSource()).join(" ")}`,
372-
loc(file, rule, sel.nodes[0]));
372+
range(file, rule, sel.nodes[0]));
373373
}
374374

375375
// Otherwise, return the block, type and associated node.

packages/@css-blocks/core/src/BlockParser/features/disallow-important.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { postcss } from "opticss";
22

33
import * as errors from "../../errors";
4-
import { sourceLocation as loc } from "../../SourceLocation";
4+
import { sourceRange } from "../../SourceLocation";
55

66
export async function disallowImportant(root: postcss.Root, file: string): Promise<postcss.Root> {
77
root.walkDecls((decl) => {
@@ -10,7 +10,7 @@ export async function disallowImportant(root: postcss.Root, file: string): Promi
1010
if (decl.important) {
1111
throw new errors.InvalidBlockSyntax(
1212
`!important is not allowed for \`${decl.prop}\` in \`${(<postcss.Rule>decl.parent).selector}\``,
13-
loc(file, decl),
13+
sourceRange(file, decl),
1414
);
1515
}
1616

packages/@css-blocks/core/src/BlockParser/features/discover-name.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { postcss } from "opticss";
22

33
import { BLOCK_NAME, CLASS_NAME_IDENT } from "../../BlockSyntax";
44
import * as errors from "../../errors";
5-
import { sourceLocation } from "../../SourceLocation";
5+
import { sourceRange } from "../../SourceLocation";
66

77
export async function discoverName(root: postcss.Root, defaultName: string, file: string): Promise<string> {
88

@@ -12,7 +12,7 @@ export async function discoverName(root: postcss.Root, defaultName: string, file
1212
if (!CLASS_NAME_IDENT.test(decl.value)) {
1313
throw new errors.InvalidBlockSyntax(
1414
`Illegal block name. '${decl.value}' is not a legal CSS identifier.`,
15-
sourceLocation(file, decl),
15+
sourceRange(file, decl),
1616
);
1717
}
1818

0 commit comments

Comments
 (0)