Commit c85342a5 authored by David Sveningsson's avatar David Sveningsson Committed by David Sveningsson

feat(rules): make options typesafe

parent 65038b9e
Pipeline #110501016 passed with stages
in 10 minutes and 36 seconds
......@@ -9,21 +9,17 @@ Rules are created by extending the `Rule` class and implementing the `setup`
method:
```typescript
// for vanilla javascript
const Rule = require("html-validate").Rule;
// for typescript
import { Rule } from "html-validate/src/rule";
import { Rule, RuleDocumentation } from "html-validate";
class MyRule extends Rule {
documentation(context?: any) {
documentation(): RuleDocumentation {
return {
description: "Lorem ipsum",
url: "https://example.net/best-practice/my-rule.html",
};
}
setup() {
setup(): void {
/* listen on dom ready event */
this.on("dom:ready", (event: DOMReadyEvent) => {
/* do something with the DOM tree */
......@@ -38,40 +34,142 @@ class MyRule extends Rule {
module.exports = MyRule;
```
For typescript generics can also be used when inheriting to specify the type of
the contextual data:
All (enabled) rules run the `setup()` callback before the source document is being parsed and is used to setup any event listeners relevant for this rule.
Many rules will use the `dom:ready` event to wait for the full DOM document to be ready but many other events are available, see {@link dev/events events} for a full list.
To report an error the rule uses the `report()` method with the DOM node and an error message.
Rules does not have to consider the severity or whenever the rule is enabled or not.
The message should be short and concise but still contain enough information to allow the user to understand what is wrong and why.
For a more verbose error (typically shown in IDEs and GUIs) the `documentation()` method is used.
This documentation might include contextual information (see below).
## Error location
By default the error is reported at the same location as the DOM node but if a better location can be provided it should be added as the third argument, typically by using the provided `sliceLocation` helper:
```typescript
interface ContextualData {
/* ... */
/* recommended: move start location by 5 characters */
const location = sliceLocation(node.location, 5);
/* not recommended: construct location manually */
const location = {
filename: node.location.filename,
line: 1,
column: 2,
offset: 1,
size: 5,
};
this.on(node, "asdf", location);
```
## Error context
Error messages may optionally include additional context.
Most CLI formatters will not include the context but JSON output and when using the API gives access to the contextual data.
This is very useful for IDE support as the short regular message might not always include enough information about why something is not allowed in a particular case.
The most important difference is that the context object is passed to the `documentation` method and can be used to give a better description of the error.
```typescript
interface RuleContext {
tagname: string;
allowed: string[];
}
class MyRule extends Rule<ContextualData> {
documentation(context?: ContextualData){ ... }
class MyRule extends Rule<RuleContext> {
/* documentation callback now accepts the optional context as first argument */
documentation(context?: RuleContext): RuleDocumentation {
/* setup the default documentation object (used when no context is available) */
const doc: RuleDocumentation = {
description: "This element cannot be used here.",
url: "https://example.net/my-rule",
};
/* if a context was passed include a better description */
if (context) {
const tagname = context.tagname;
const allowed = context.allowed.join(", ");
doc.description = `The ${tagname} element cannot be used here, only one of ${allowed} is allowed.`;
}
setup(){
const context: ContextualData = { .. };
this.report(node, "Message", null, context);
return doc;
}
setup(): void {
/* actual setup code left out for brevity */
/* create a context object for this error */
const context: RuleContext = {
tagname: node.tagName,
allowed: ["<b>", "<i>", "<u>"],
};
/* pass the context when reporting an error */
this.report(node, "This element cannot be used here", null, context);
}
}
```
<div class="alert alert-info">
<i class="fa fa-info-circle" aria-hidden="true"></i>
<strong>Note</strong>
<p>Even if your rule reports contextual data the API user might not pass it back to the <code>documentation()</code> call so you must always test if the context object was actually passed or not.</p>
</div>
## Options
If the rule has options create an interface and pass as the second template argument.
Options can either be accessed in the class constructor or on `this.options`.
If any option is required be use to include defaults as the use must not be required to enter any options in their configuration.
```typescript
interface RuleOptions {
text: string;
}
const defaults: RuleOptions = {
text: "lorem ipsum",
};
class MyRule extends Rule<void, RuleOptions> {
constructor(options: RuleOptions) {
/* assign default values if not provided by user */
super(Object.assign({}, defaults, options));
}
setup(): void {
/* actual setup code left out for brevity */
/* disallow the node from containing the text provided in the option */
if (node.textContent.inclues(this.options.text)) {
this.report(node, "Contains disallowed text");
}
}
}
```
## API
### `options: {[key: string]: any}`
### `options: RuleOptions`
Object with all the options passed from the configuration.
Options are accessed using `this.options`.
When using typescript: pass the datatype as the second template argument when extending `Rule`.
Default is `void` (i.e. no options)
### `on(event: string, callback: (event: Event)): void`
Listen for events. See [events](/dev/events.html) for a full list of available
events and data.
### `report(node: HtmlElement, message: string, location?: Location, context?: T): void`
### `report(node: DOMNode, message: string, location?: Location, context?: RuleContext): void`
Report a new error.
- `node` - The `HtmlElement` this error belongs to.
- `node` - The `DOMNode` this error belongs to.
- `message` - Error message
- _`location`_ - If set it is the precise location of the error. (Default: node
location)
......
......@@ -6,8 +6,8 @@ import "../matchers";
import { MetaTable } from "../meta";
import { Parser, ParserError } from "../parser";
import { Reporter } from "../reporter";
import { Rule, RuleOptions } from "../rule";
import { Engine } from "./engine";
import { Rule } from "../rule";
import { Engine, RuleOptions } from "./engine";
function inline(source: string): Source {
return {
......
......@@ -10,7 +10,9 @@ import {
import { InvalidTokenError, Lexer, TokenType } from "../lexer";
import { Parser, ParserError } from "../parser";
import { Report, Reporter } from "../reporter";
import { Rule, RuleConstructor, RuleDocumentation, RuleOptions } from "../rule";
import { Rule, RuleConstructor, RuleDocumentation } from "../rule";
export type RuleOptions = Record<string, any>;
export interface EventDump {
event: string;
......@@ -426,7 +428,7 @@ export class Engine<T extends Parser = Parser> {
this.report(null, `Definition for rule '${name}' was not found`);
});
}
})({});
})();
}
private reportError(
......
export function parsePattern(pattern: string): RegExp {
export type PatternName = "kebabcase" | "camelcase" | "underscore" | string;
export function parsePattern(pattern: PatternName): RegExp {
switch (pattern) {
case "kebabcase":
return /^[a-z0-9-]+$/;
......@@ -15,7 +17,7 @@ export function parsePattern(pattern: string): RegExp {
}
}
export function describePattern(pattern: string): string {
export function describePattern(pattern: PatternName): string {
const regexp = parsePattern(pattern).toString();
switch (pattern) {
case "kebabcase":
......
......@@ -296,7 +296,7 @@ describe("Plugin", () => {
public setup(): void {
/* do nothing */
}
})({});
})();
mockPlugin.rules = {
"mock-rule": null /* instantiateRule is mocked, this can be anything */,
};
......
......@@ -100,12 +100,12 @@ export class Reporter {
};
}
public add(
rule: Rule,
public add<ContextType, OptionsType>(
rule: Rule<ContextType, OptionsType>,
message: string,
severity: number,
location: Location,
context?: any
context?: ContextType
): void {
if (!(location.filename in this.result)) {
this.result[location.filename] = [];
......
......@@ -8,7 +8,11 @@ import { Reporter } from "./reporter";
import { Rule, ruleDocumentationUrl } from "./rule";
import { MetaTable } from "./meta";
class MockRule extends Rule {
interface RuleContext {
foo: string;
}
class MockRule extends Rule<RuleContext> {
public setup(): void {
/* do nothing */
}
......@@ -18,7 +22,7 @@ describe("rule base class", () => {
let parser: Parser;
let reporter: Reporter;
let meta: MetaTable;
let rule: Rule;
let rule: MockRule;
let mockLocation: Location;
let mockEvent: Event;
......@@ -30,7 +34,7 @@ describe("rule base class", () => {
meta = new MetaTable();
meta.loadFromFile(path.join(__dirname, "../elements/html5.json"));
rule = new MockRule({});
rule = new MockRule();
rule.name = "mock-rule";
rule.init(parser, reporter, Severity.ERROR, meta);
mockLocation = { filename: "mock-file", offset: 1, line: 1, column: 2 };
......
......@@ -20,18 +20,14 @@ import { MetaTable, MetaLookupableProperty } from "./meta";
const homepage = require("../package.json").homepage;
export interface RuleOptions {
[key: string]: any;
}
export interface RuleDocumentation {
description: string;
url?: string;
}
export type RuleConstructor = new (options: RuleOptions) => Rule;
export type RuleConstructor = new (options?: any) => Rule;
export abstract class Rule<T = any> {
export abstract class Rule<ContextType = void, OptionsType = void> {
private reporter: Reporter;
private parser: Parser;
private meta: MetaTable;
......@@ -48,9 +44,9 @@ export abstract class Rule<T = any> {
/**
* Rule options.
*/
public readonly options: RuleOptions;
public readonly options: OptionsType;
public constructor(options: RuleOptions) {
public constructor(options: OptionsType) {
this.options = options;
this.enabled = true;
}
......@@ -100,7 +96,7 @@ export abstract class Rule<T = any> {
node: DOMNode,
message: string,
location?: Location,
context?: T
context?: ContextType
): void {
if (this.isEnabled() && (!node || node.ruleEnabled(this.name))) {
const where = this.findLocation({ node, location, event: this.event });
......@@ -202,7 +198,7 @@ export abstract class Rule<T = any> {
* additional documentation is available.
*/
/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
public documentation(context?: T): RuleDocumentation {
public documentation(context?: ContextType): RuleDocumentation {
return null;
}
}
......
import { HtmlElement } from "../dom";
import { AttributeEvent } from "../event";
import { Rule, RuleDocumentation, ruleDocumentationUrl } from "../rule";
import { CaseStyle } from "./helper/case-style";
import { CaseStyle, CaseStyleName } from "./helper/case-style";
const defaults = {
interface RuleOptions {
style: CaseStyleName | CaseStyleName[];
ignoreForeign: boolean;
}
const defaults: RuleOptions = {
style: "lowercase",
ignoreForeign: true,
};
class AttrCase extends Rule {
class AttrCase extends Rule<void, RuleOptions> {
private style: CaseStyle;
public constructor(options: object) {
public constructor(options: RuleOptions) {
super(Object.assign({}, defaults, options));
this.style = new CaseStyle(this.options.style, "attr-case");
}
......
......@@ -8,12 +8,17 @@ enum QuoteStyle {
AUTO_QUOTE = "auto",
}
const defaults = {
interface Options {
style?: '"' | "'" | "auto";
unquoted?: boolean;
}
const defaults: Options = {
style: "auto",
unquoted: false,
};
class AttrQuotes extends Rule {
class AttrQuotes extends Rule<void, Options> {
private style: QuoteStyle;
public documentation(): RuleDocumentation {
......@@ -30,7 +35,7 @@ class AttrQuotes extends Rule {
}
}
public constructor(options: object) {
public constructor(options: Options) {
super(Object.assign({}, defaults, options));
this.style = parseStyle(this.options.style);
}
......
......@@ -3,16 +3,20 @@ import { DOMReadyEvent } from "../event";
import { PermittedAttribute } from "../meta/element";
import { Rule, RuleDocumentation, ruleDocumentationUrl } from "../rule";
const defaults = {
interface Options {
style?: string;
}
const defaults: Options = {
style: "omit",
};
type checkFunction = (attr: Attribute) => boolean;
class AttributeBooleanStyle extends Rule {
class AttributeBooleanStyle extends Rule<void, Options> {
private hasInvalidStyle: checkFunction;
public constructor(options: object) {
public constructor(options: Options) {
super(Object.assign({}, defaults, options));
this.hasInvalidStyle = parseStyle(this.options.style);
}
......
import { DOMTokenList } from "../dom";
import { AttributeEvent } from "../event";
import { describePattern, parsePattern } from "../pattern";
import { describePattern, parsePattern, PatternName } from "../pattern";
import { Rule, RuleDocumentation, ruleDocumentationUrl } from "../rule";
const defaults = {
interface RuleOptions {
pattern: PatternName;
}
const defaults: RuleOptions = {
pattern: "kebabcase",
};
class ClassPattern extends Rule {
class ClassPattern extends Rule<void, RuleOptions> {
private pattern: RegExp;
public constructor(options: object) {
public constructor(options: RuleOptions) {
super(Object.assign({}, defaults, options));
this.pattern = parsePattern(this.options.pattern);
}
......
......@@ -2,16 +2,20 @@ import { Location, sliceLocation } from "../context";
import { HtmlElement } from "../dom";
import { TagCloseEvent, TagOpenEvent } from "../event";
import { Rule, RuleDocumentation, ruleDocumentationUrl } from "../rule";
import { CaseStyle } from "./helper/case-style";
import { CaseStyle, CaseStyleName } from "./helper/case-style";
const defaults = {
interface RuleOptions {
style: CaseStyleName;
}
const defaults: RuleOptions = {
style: "lowercase",
};
class ElementCase extends Rule {
class ElementCase extends Rule<void, RuleOptions> {
private style: CaseStyle;
public constructor(options: object) {
public constructor(options: RuleOptions) {
super(Object.assign({}, defaults, options));
this.style = new CaseStyle(this.options.style, "element-case");
}
......
......@@ -2,16 +2,22 @@ import { sliceLocation } from "../context";
import { TagOpenEvent } from "../event";
import { Rule, RuleDocumentation, ruleDocumentationUrl } from "../rule";
const defaults = {
interface RuleOptions {
pattern: string;
whitelist: string[];
blacklist: string[];
}
const defaults: RuleOptions = {
pattern: "^[a-z][a-z0-9\\-._]*-[a-z0-9\\-._]*$",
whitelist: [] as string[],
blacklist: [] as string[],
whitelist: [],
blacklist: [],
};
class ElementName extends Rule {
class ElementName extends Rule<void, RuleOptions> {
private pattern: RegExp;
public constructor(options: object) {
public constructor(options: RuleOptions) {
super(Object.assign({}, defaults, options));
// eslint-disable-next-line security/detect-non-literal-regexp
......
......@@ -39,7 +39,7 @@ it("should handle multiple patterns", () => {
it("should throw exception for unknown styles", () => {
expect.assertions(1);
expect(() => {
return new CaseStyle("unknown-style", "test-case");
return new CaseStyle("unknown-style" as any, "test-case");
}).toThrow('Invalid style "unknown-style" for test-case rule');
});
......
import { ConfigError } from "../../config/error";
export type CaseStyleName =
| "lowercase"
| "uppercase"
| "pascalcase"
| "camelcase";
interface Style {
pattern: RegExp;
name: string;
......@@ -14,7 +20,7 @@ export class CaseStyle {
/**
* @param style - Name of a valid case style.
*/
public constructor(style: string | string[], ruleId: string) {
public constructor(style: CaseStyleName | CaseStyleName[], ruleId: string) {
if (!Array.isArray(style)) {
style = [style];
}
......@@ -46,7 +52,7 @@ export class CaseStyle {
}
}
private parseStyle(style: string[], ruleId: string): Style[] {
private parseStyle(style: CaseStyleName[], ruleId: string): Style[] {
return style.map(
(cur: string): Style => {
switch (cur.toLowerCase()) {
......
import { DynamicValue } from "../dom";
import { AttributeEvent } from "../event";
import { describePattern, parsePattern } from "../pattern";
import { describePattern, parsePattern, PatternName } from "../pattern";
import { Rule, RuleDocumentation, ruleDocumentationUrl } from "../rule";
const defaults = {
interface RuleOptions {
pattern: PatternName;
}
const defaults: RuleOptions = {
pattern: "kebabcase",
};
class IdPattern extends Rule {
class IdPattern extends Rule<void, RuleOptions> {
private pattern: RegExp;
public constructor(options: object) {
public constructor(options: RuleOptions) {
super(Object.assign({}, defaults, options));
this.pattern = parsePattern(this.options.pattern);
}
......
import { Rule, RuleDocumentation, ruleDocumentationUrl } from "../rule";
const defaults = {
interface RuleOptions {
maxlength: number;
}
const defaults: RuleOptions = {
maxlength: 70,
};
class LongTitle extends Rule {
class LongTitle extends Rule<void, RuleOptions> {
private maxlength: number;
public constructor(options: object) {
public constructor(options: RuleOptions) {
super(Object.assign({}, defaults, options));
this.maxlength = parseInt(this.options.maxlength, 10);
this.maxlength = this.options.maxlength;
}
public documentation(): RuleDocumentation {
......
......@@ -3,7 +3,11 @@ import { NodeType } from "../dom";
import { AttributeEvent, ElementReadyEvent } from "../event";
import { Rule, RuleDocumentation, ruleDocumentationUrl } from "../rule";
const defaults = {