Commit fe1705e1 authored by David Sveningsson's avatar David Sveningsson

feat(htmlvalidate): retain `offset` when yielding multiple sources

If a transformer yields multiple sources the original `offset` should not be
reset to 0 for each one but rather retain the original offset similar to
lines/columns. Since the original caller has no way to know where the
transformer has yielded sources a user relying on `offset` has no way of knowing
the correct positions if it is reset.
parent 6d05747d
......@@ -338,6 +338,7 @@ describe("config", () => {
data: "original data",
line: 2,
column: 3,
offset: 4,
};
});
......@@ -355,6 +356,7 @@ describe("config", () => {
"data": "transformed source (was: original data)",
"filename": "/path/to/test.foo",
"line": 1,
"offset": 0,
"originalData": "original data",
},
]
......@@ -410,6 +412,7 @@ describe("config", () => {
"data": "transformed from foo.unnamed",
"filename": "foo.unnamed",
"line": 2,
"offset": 4,
},
]
`);
......@@ -425,6 +428,7 @@ describe("config", () => {
"data": "transformed from bar.named",
"filename": "bar.named",
"line": 2,
"offset": 4,
},
]
`);
......@@ -440,6 +444,7 @@ describe("config", () => {
"data": "transformed source (was: original data)",
"filename": "bar.nonplugin",
"line": 1,
"offset": 0,
"originalData": "original data",
},
]
......@@ -472,6 +477,7 @@ describe("config", () => {
"data": "original data",
"filename": "/path/to/test.foo",
"line": 2,
"offset": 4,
},
]
`);
......@@ -493,6 +499,7 @@ describe("config", () => {
"data": "transformed source (was: data from mock-transform-chain-foo (was: original data))",
"filename": "/path/to/test.bar",
"line": 1,
"offset": 0,
"originalData": "original data",
},
]
......@@ -515,6 +522,7 @@ describe("config", () => {
"data": "transformed source (was: data from mock-transform-optional-chain (was: original data))",
"filename": "/path/to/test.bar.foo",
"line": 1,
"offset": 0,
"originalData": "original data",
},
]
......@@ -537,6 +545,7 @@ describe("config", () => {
"data": "transformed source (was: original data)",
"filename": "/path/to/test.foo",
"line": 1,
"offset": 0,
"originalData": "original data",
},
]
......@@ -605,6 +614,7 @@ describe("config", () => {
",
"filename": "test-files/parser/simple.html",
"line": 1,
"offset": 0,
"originalData": "<p>Lorem ipsum</p>
",
},
......
......@@ -387,6 +387,7 @@ export class Config {
filename,
line: 1,
column: 1,
offset: 0,
originalData: data,
};
return this.transformSource(source);
......
......@@ -19,7 +19,7 @@ export class Context {
this.state = undefined;
this.string = source.data;
this.filename = source.filename;
this.offset = 0;
this.offset = source.offset;
this.line = source.line;
this.column = source.column;
this.contentModel = ContentModel.TEXT;
......
......@@ -39,8 +39,37 @@ export interface SourceHooks {
export interface Source {
data: string;
filename: string;
/**
* Line in the original data.
*
* Starts at 1 (first line).
*/
line: number;
/**
* Column in the original data.
*
* Starts at 1 (first column).
*/
column: number;
/**
* Offset in the original data.
*
* Starts at 0 (first character).
*/
offset: number;
/**
* Original data. When a transformer extracts a portion of the original source
* this must be set to the full original source.
*
* Since the transformer might be chained always test if the input source
* itself has `originalData` set, e.g.:
*
* `originalData = input.originalData || input.data`.
*/
originalData?: string;
/**
......
......@@ -41,6 +41,7 @@ describe("HtmlElement", () => {
filename: "inline",
line: 1,
column: 1,
offset: 0,
hooks: {
processAttribute,
},
......
......@@ -14,6 +14,7 @@ function inline(source: string): Source {
filename: "inline",
line: 1,
column: 1,
offset: 0,
data: source,
};
}
......
......@@ -24,10 +24,11 @@ function mockConfig(): Config {
config.init();
config.transformFilename = jest.fn((filename: string) => [
{
column: 1,
data: `source from ${filename}`,
filename,
line: 1,
column: 1,
offset: 0,
},
]);
return config;
......@@ -70,6 +71,7 @@ describe("HtmlValidate", () => {
data: str,
filename: "inline",
line: 1,
offset: 0,
},
]);
});
......@@ -94,6 +96,7 @@ describe("HtmlValidate", () => {
filename: "inline",
line: 1,
column: 1,
offset: 0,
};
const report = htmlvalidate.validateSource(source);
expect(report).toEqual(mockReport);
......@@ -114,6 +117,7 @@ describe("HtmlValidate", () => {
data: "source from foo.html",
filename,
line: 1,
offset: 0,
},
]);
});
......@@ -243,6 +247,7 @@ describe("HtmlValidate", () => {
data: "source from foo.html",
filename,
line: 1,
offset: 0,
},
]);
});
......@@ -258,6 +263,7 @@ describe("HtmlValidate", () => {
data: "source from foo.html",
filename,
line: 1,
offset: 0,
},
]);
});
......@@ -273,6 +279,7 @@ describe("HtmlValidate", () => {
data: "source from foo.html",
filename,
line: 1,
offset: 0,
},
]);
});
......@@ -284,16 +291,18 @@ describe("HtmlValidate", () => {
config.init();
config.transformFilename = jest.fn((filename: string) => [
{
column: 1,
data: `first markup`,
filename,
line: 1,
column: 1,
offset: 0,
},
{
column: 3,
data: `second markup`,
filename,
line: 5,
column: 3,
offset: 29,
},
]);
jest.spyOn(htmlvalidate, "getConfigFor").mockImplementation(() => config);
......@@ -376,6 +385,7 @@ describe("HtmlValidate", () => {
filename: "inline",
line: 1,
column: 1,
offset: 0,
};
const parser = htmlvalidate.getParserFor(source);
expect(parser).toBeInstanceOf(Parser);
......
......@@ -43,10 +43,11 @@ class HtmlValidate {
hooks?: SourceHooks
): Report {
const source = {
column: 1,
data: str,
filename: filename || "inline",
line: 1,
column: 1,
offset: 0,
hooks,
};
return this.validateSource(source);
......
import HtmlValidate from "./htmlvalidate";
import { Source } from "./context";
import { TRANSFORMER_API } from "./transform";
import { Plugin } from "./plugin";
import "./matchers";
import { Rule } from "./rule";
import { DOMReadyEvent } from "./event";
it("should compute correct line, column and offset when using transformed sources", () => {
expect.assertions(2);
/* create a mock rule which reports error on root element */
class MockRule extends Rule {
public setup(): void {
this.on("dom:ready", (event: DOMReadyEvent) => {
const root = event.document.root;
this.report(root, "mock error");
});
}
}
/* create a mock transformer which will create a new source for each line */
function transformer(source: Source): Source[] {
const lines = source.data.split("\n");
return lines.filter(Boolean).map(
(line: string, index: number): Source => {
/* all lines have the same length */
const offset = (line.length + 1) * index;
return {
data: line,
filename: source.filename,
line: index + 1,
column: 1,
offset,
originalData: source.data,
};
}
);
}
transformer.api = TRANSFORMER_API.VERSION;
/* create a mock plugin to expose mocks */
const plugin: Plugin = {
rules: {
"mock-rule": MockRule,
},
transformer,
};
jest.mock("plugin", () => plugin, { virtual: true });
/* create validator instance configured to use mock */
const htmlvalidate = new HtmlValidate({
root: true,
plugins: ["plugin"],
transform: {
".*": "plugin",
},
rules: {
"mock-rule": "error",
},
});
/* ensure line, column and offsets are correct */
const report = htmlvalidate.validateString(
"<p>line 1</p>\n<p>line 2</p>\n<p>line 3</p>\n"
);
expect(report).toBeInvalid();
expect(report.results[0]).toMatchInlineSnapshot(`
Object {
"errorCount": 3,
"filePath": "inline",
"messages": Array [
Object {
"column": 1,
"context": undefined,
"line": 1,
"message": "mock error",
"offset": 0,
"ruleId": "mock-rule",
"severity": 2,
"size": 0,
},
Object {
"column": 1,
"context": undefined,
"line": 2,
"message": "mock error",
"offset": 14,
"ruleId": "mock-rule",
"severity": 2,
"size": 0,
},
Object {
"column": 1,
"context": undefined,
"line": 3,
"message": "mock error",
"offset": 28,
"ruleId": "mock-rule",
"severity": 2,
"size": 0,
},
],
"source": "<p>line 1</p>
<p>line 2</p>
<p>line 3</p>
",
"warningCount": 0,
}
`);
});
......@@ -9,6 +9,7 @@ function inlineSource(source: string, { line = 1, column = 1 } = {}): Source {
filename: "inline",
line,
column,
offset: 0,
};
}
......
......@@ -900,6 +900,7 @@ describe("parser", () => {
filename: "inline",
line: 1,
column: 1,
offset: 0,
hooks: {
processAttribute,
},
......@@ -952,6 +953,7 @@ describe("parser", () => {
filename: "inline",
line: 1,
column: 1,
offset: 0,
hooks: {
processAttribute,
},
......@@ -988,6 +990,7 @@ describe("parser", () => {
filename: "inline",
line: 1,
column: 1,
offset: 0,
hooks: {
processElement,
},
......
......@@ -55,13 +55,14 @@ export class Parser {
filename: "inline",
line: 1,
column: 1,
offset: 0,
};
}
/* reset DOM in case there are multiple calls in the same session */
this.dom = new DOMTree({
filename: source.filename,
offset: 0,
offset: source.offset,
line: source.line,
column: source.column,
});
......
......@@ -21,6 +21,7 @@ describe("Plugin", () => {
filename: "inline",
line: 1,
column: 1,
offset: 0,
};
/* reset mock */
......@@ -284,6 +285,7 @@ describe("Plugin", () => {
filename: source.filename,
line: source.line,
column: source.column,
offset: source.offset,
originalData: source.data,
},
];
......@@ -302,6 +304,7 @@ describe("Plugin", () => {
filename: "/path/to/mock.filename",
line: 2,
column: 3,
offset: 4,
});
expect(sources).toMatchInlineSnapshot(`
Array [
......@@ -310,6 +313,7 @@ describe("Plugin", () => {
"data": "transformed from unnamed transformer",
"filename": "/path/to/mock.filename",
"line": 2,
"offset": 4,
"originalData": "original data",
},
]
......@@ -325,6 +329,7 @@ describe("Plugin", () => {
filename: source.filename,
line: source.line,
column: source.column,
offset: source.offset,
originalData: source.data,
},
];
......@@ -345,6 +350,7 @@ describe("Plugin", () => {
filename: "/path/to/mock.filename",
line: 2,
column: 3,
offset: 4,
});
expect(sources).toMatchInlineSnapshot(`
Array [
......@@ -353,6 +359,7 @@ describe("Plugin", () => {
"data": "transformed from named transformer",
"filename": "/path/to/mock.filename",
"line": 2,
"offset": 4,
"originalData": "original data",
},
]
......
......@@ -169,13 +169,20 @@ describe("Reporter", () => {
it("should map filenames to sources", () => {
const report = new Reporter();
const sources: Source[] = [
{ filename: "foo.html", data: "<foo></foo>", line: 1, column: 1 },
{
filename: "foo.html",
data: "<foo></foo>",
line: 1,
column: 1,
offset: 0,
},
{
filename: "bar.html",
data: "transformed",
originalData: "<bar></bar>",
line: 1,
column: 1,
offset: 0,
},
];
report.addManual("foo.html", createMessage("error", 1));
......
......@@ -17,6 +17,7 @@ describe("a17y helpers", () => {
filename: "inline",
line: 1,
column: 1,
offset: 0,
hooks: {
processAttribute,
},
......
......@@ -14,6 +14,7 @@ function* mockTransformChainFoo(
filename: source.filename,
line: 1,
column: 1,
offset: 0,
originalData: source.originalData || source.data,
},
`${source.filename}.foo`
......
......@@ -17,6 +17,7 @@ function* mockTransformOptionalChain(
filename: source.filename,
line: 1,
column: 1,
offset: 0,
originalData: source.originalData || source.data,
},
next
......
......@@ -11,6 +11,7 @@ function mockTransform(source: Source): Iterable<Source> {
filename: source.filename,
line: 1,
column: 1,
offset: 0,
originalData: source.originalData || source.data,
},
];
......
......@@ -5,7 +5,13 @@ describe("TemplateExtractor", () => {
it("should extract templates from object property", () => {
const te = TemplateExtractor.fromString('foo({template: "<b>foo</b>"})');
expect(te.extractObjectProperty("template")).toEqual([
{ data: "<b>foo</b>", filename: "inline", line: 1, column: 16 },
{
data: "<b>foo</b>",
filename: "inline",
line: 1,
column: 16,
offset: 16,
},
]);
});
......@@ -14,7 +20,13 @@ describe("TemplateExtractor", () => {
'foo({"template": "<b>foo</b>"})'
);
expect(te.extractObjectProperty("template")).toEqual([
{ data: "<b>foo</b>", filename: "inline", line: 1, column: 18 },
{
data: "<b>foo</b>",
filename: "inline",
line: 1,
column: 18,
offset: 18,
},
]);
});
......@@ -26,14 +38,26 @@ describe("TemplateExtractor", () => {
it("should handle single quotes", () => {
const te = TemplateExtractor.fromString("foo({template: '<b>foo</b>'})");
expect(te.extractObjectProperty("template")).toEqual([
{ data: "<b>foo</b>", filename: "inline", line: 1, column: 16 },
{
data: "<b>foo</b>",
filename: "inline",
line: 1,
column: 16,
offset: 16,
},
]);
});
it("should handle double quotes", () => {
const te = TemplateExtractor.fromString('foo({template: "<b>foo</b>"})');
expect(te.extractObjectProperty("template")).toEqual([
{ data: "<b>foo</b>", filename: "inline", line: 1, column: 16 },
{
data: "<b>foo</b>",
filename: "inline",
line: 1,
column: 16,
offset: 16,
},
]);
});
......@@ -42,7 +66,13 @@ describe("TemplateExtractor", () => {
"foo({template: `<b>${foo}</b>`})"
);
expect(te.extractObjectProperty("template")).toEqual([
{ data: "<b> </b>", filename: "inline", line: 1, column: 16 },
{
data: "<b> </b>",
filename: "inline",
line: 1,
column: 16,
offset: 16,
},
]);
});
......@@ -51,7 +81,13 @@ describe("TemplateExtractor", () => {
"foo({template: foo`<b>${foo}</b>`})"
);
expect(te.extractObjectProperty("template")).toEqual([
{ data: "<b> </b>", filename: "inline", line: 1, column: 19 },
{
data: "<b> </b>",
filename: "inline",
line: 1,
column: 19,
offset: 19,
},
]);
});
......@@ -60,7 +96,13 @@ describe("TemplateExtractor", () => {
"foo({template: (foo) => `<b>${foo}</b>`})"
);
expect(te.extractObjectProperty("template")).toEqual([
{ data: "<b> </b>", filename: "inline", line: 1, column: 25 },
{
data: "<b> </b>",
filename: "inline",
line: 1,
column: 25,
offset: 25,