Commit fcad0b2a authored by David Sveningsson's avatar David Sveningsson
Browse files

fix(config): dont automatically apply `extends: ["html-validate:recommended"]`

fixes #98

BREAKING CHANGE: With this release any custom configuration files will no longer
automatically extend `html-validate:recommended`.

The previous behaviour was to to always merge with the default configuration
containing `extends: ["html-validate:recommended"]`. This is counter-intuitive
when adding a blank `{}` `.htmlvalidate.json` configuration file (or a file with
`extends: []`). The new behaviour is to not apply default configuration if
another configuration is found.

To retain the previous behaviour you must ensure your configuration contains
`extends: ["html-validate:recommended"]`. Users who have `root: true` are not
affected. *If unsure: test your configuration by deliberatly introducing and
error and verify it is detected*.

For CLI users: ensure your `.htmlvalidate.json` configuration files are updated.

For IDE integration users: ensure your `.htmlvalidate.json` configuration files
are updated.

For `CLI` API users: same as with CLI the configuration loading has changed and
all users must update their `.htmlvalidate.json` accordingly.

For `HtmlValidate` API users: you are most likely not affected, only if both of
the following conditions are true you need to take care to ensure any
`.htmlvalidate.json` is updated:

1. you are using `validateFile` or supply filenames to other validation
   functions.
2. you allow user-supplied configurations (or use them yourself) via
   `.htmlvalidate.json` (default unless `root: true` is set in the configuration
   object passed to `new HtmlValidate(..)`)

The `ConfigLoader` API has also been updated and `fromTarget` may now return
`null` if no configuration file was found.
parent aef181ba
Pipeline #212998813 passed with stages
in 9 minutes and 57 seconds
......@@ -23,8 +23,7 @@ if (!report.valid) {
files, load plugins, runs any transformations etc and is very similar to using
the CLI tool (in fact, the CLI tool uses this very API).
A default configuration object may optionally be passed to the `HtmlValidate`
constructor:
A configuration object may optionally be passed to the `HtmlValidate` constructor:
```typescript
const htmlvalidate = new HtmlValidate({
......@@ -32,7 +31,8 @@ const htmlvalidate = new HtmlValidate({
});
```
If set, all configuration will inherit from this configuration.
If set, it will be used as configuration unless a configuration could be read from `.htmlvalidate.json` files.
Set `root: true` to prevent configuration files to be searched.
## Validating strings and other sources
......
......@@ -120,10 +120,10 @@ describe("ConfigLoader", () => {
);
});
it("should load empty config for inline sources", () => {
it("should return null for inline sources", () => {
expect.assertions(1);
const config = loader.fromTarget("inline");
expect(config.get()).toEqual(Config.empty().get());
expect(config).toBeNull();
});
it("should cache results", () => {
......
......@@ -17,14 +17,14 @@ interface ConfigClass {
* parent directories is searched as well and the result is merged together.
*/
export class ConfigLoader {
protected cache: Map<string, Config>;
protected cache: Map<string, Config | null>;
protected configClass: ConfigClass;
/**
* @param configClass - Override class to construct.
*/
public constructor(configClass: ConfigClass) {
this.cache = new Map<string, Config>();
this.cache = new Map<string, Config | null>();
this.configClass = configClass;
}
......@@ -48,15 +48,16 @@ export class ConfigLoader {
*
* @param filename Filename to get configuration for.
*/
public fromTarget(filename: string): Config {
public fromTarget(filename: string): Config | null {
if (filename === "inline") {
return this.configClass.empty();
return null;
}
if (this.cache.has(filename)) {
return this.cache.get(filename);
}
let found = false;
let current = path.resolve(path.dirname(filename));
let config = this.configClass.empty();
......@@ -66,6 +67,7 @@ export class ConfigLoader {
if (fs.existsSync(search)) {
const local = this.configClass.fromFile(search);
found = true;
config = local.merge(config);
}
......@@ -84,6 +86,12 @@ export class ConfigLoader {
}
}
/* no config was found by loader, return null and let caller decide what to do */
if (!found) {
this.cache.set(filename, null);
return null;
}
this.cache.set(filename, config);
return config;
}
......
......@@ -71,8 +71,8 @@ describe("config", () => {
expect.assertions(1);
const config = Config.defaultConfig();
expect(config.get()).toEqual({
extends: ["html-validate:recommended"],
rules: expect.any(Object),
extends: [],
rules: {},
plugins: [],
transform: {},
});
......
export default {
extends: ["html-validate:recommended"],
};
export default {};
......@@ -42,21 +42,25 @@ describe("HtmlValidate", () => {
it("should load default config if no configuration was passed", () => {
expect.assertions(1);
const htmlvalidate = new HtmlValidate();
expect((htmlvalidate as any).globalConfig.config).toEqual(
expect.objectContaining({
extends: ["html-validate:recommended"],
})
);
expect((htmlvalidate as any).globalConfig.config).toEqual({
extends: [],
plugins: [],
rules: {},
transform: {},
});
});
it("should not load default config if configuration was passed", () => {
expect.assertions(1);
const htmlvalidate = new HtmlValidate({});
expect((htmlvalidate as any).globalConfig.config).toEqual(
expect.objectContaining({
extends: [],
})
);
const htmlvalidate = new HtmlValidate({ rules: { foo: "error" } });
expect((htmlvalidate as any).globalConfig.config).toEqual({
extends: [],
plugins: [],
rules: {
foo: "error",
},
transform: {},
});
});
describe("validateString()", () => {
......@@ -542,71 +546,126 @@ describe("HtmlValidate", () => {
});
describe("getConfigFor()", () => {
it("should load configuration files and merge result in correct order", () => {
expect.assertions(2);
it("should query configuration loader with passed filename", () => {
expect.assertions(1);
const htmlvalidate = new HtmlValidate();
const fromTarget = jest
.spyOn((htmlvalidate as any).configLoader, "fromTarget")
.mockImplementation(() => null);
htmlvalidate.getConfigFor("my-file.html");
expect(fromTarget).toHaveBeenCalledWith("my-file.html");
});
it("should use global configuration by default", () => {
expect.assertions(1);
/* constructor global config */
const htmlvalidate = new HtmlValidate({
rules: {
a: "error",
b: "error",
},
});
const fromTarget = jest
.spyOn((htmlvalidate as any).configLoader, "fromTarget")
.mockImplementation(() =>
Config.fromObject({
rules: {
a: "warn",
c: "warn",
},
})
);
/* .htmlvalidate.json */
jest.spyOn((htmlvalidate as any).configLoader, "fromTarget").mockImplementation(() => null);
const config = htmlvalidate.getConfigFor("my-file.html");
expect(fromTarget).toHaveBeenCalledWith("my-file.html");
expect(config.get()).toEqual(
expect.objectContaining({
expect(config.get()).toEqual({
extends: [],
plugins: [],
rules: {
a: "error",
},
transform: {},
});
});
it("should merge global configuration with override if provided", () => {
expect.assertions(1);
/* constructor global config */
const htmlvalidate = new HtmlValidate({
rules: {
a: "error",
b: "error",
},
});
/* .htmlvalidate.json */
jest.spyOn((htmlvalidate as any).configLoader, "fromTarget").mockImplementation(() => null);
/* override */
const config = htmlvalidate.getConfigFor("my-file.html", {
rules: {
a: "warn",
c: "warn",
},
});
expect(config.get()).toEqual({
extends: [],
plugins: [],
rules: {
a: "warn",
b: "error",
c: "warn",
},
transform: {},
});
});
it("should use configuration provided by configuration loader if present", () => {
expect.assertions(1);
/* constructor global config */
const htmlvalidate = new HtmlValidate({
rules: {
a: "error",
},
});
/* .htmlvalidate.json */
jest.spyOn((htmlvalidate as any).configLoader, "fromTarget").mockImplementation(() =>
Config.fromObject({
rules: {
a: "warn",
b: "error",
c: "warn",
},
})
);
const config = htmlvalidate.getConfigFor("my-file.html");
expect(config.get()).toEqual({
extends: [],
plugins: [],
rules: {
b: "error",
},
transform: {},
});
});
it("should apply configuration override in correct order", () => {
it("should merge configuration provided by configuration loader with override if provided", () => {
expect.assertions(1);
/* constructor global config */
const htmlvalidate = new HtmlValidate({
rules: {
a: "error",
b: "error",
c: "error",
},
});
/* .htmlvalidate.json */
jest.spyOn((htmlvalidate as any).configLoader, "fromTarget").mockImplementation(() =>
Config.fromObject({
rules: {
a: "warn",
b: "warn",
b: "error",
c: "error",
},
})
);
/* override */
const config = htmlvalidate.getConfigFor("my-file.html", {
rules: {
a: "off",
b: "warn",
},
});
expect(config.get()).toEqual(
expect.objectContaining({
rules: {
a: "off",
b: "warn",
c: "error",
},
})
);
expect(config.get()).toEqual({
extends: [],
plugins: [],
rules: {
b: "warn",
c: "error",
},
transform: {},
});
});
it("should not load configuration files if global config is root", () => {
......
......@@ -269,6 +269,9 @@ class HtmlValidate {
* 2. `.htmlvalidate.json` files found when traversing the directory structure.
* 3. Override passed to this function.
*
* Global configuration is used when no `.htmlvalidate.json` is found. The
* result is always merged with override if present.
*
* The `root` property set to `true` affects the configuration as following:
*
* 1. If set in override the override is returned as-is.
......@@ -297,7 +300,7 @@ class HtmlValidate {
}
const config = this.configLoader.fromTarget(filename);
const merged = this.globalConfig.merge(config).merge(override);
const merged = config ? config.merge(override) : this.globalConfig.merge(override);
merged.init();
return merged;
}
......
jest.mock("./config/default", () => {
return {
extends: ["html-validate:recommended"],
};
});
import stripAnsi = require("strip-ansi");
import { Severity } from "./config";
import { Token, TokenType } from "./lexer";
......
{
"root": true,
"extends": ["html-validate:recommended"],
"rules": {
"void-content": "error"
}
......
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`cli-default-config/test.html 1`] = `
Array [
Object {
"column": 8,
"context": undefined,
"line": 1,
"message": "Mismatched close-tag, expected '</p>' but found '</i>'.",
"offset": 7,
"ruleId": "close-order",
"selector": null,
"severity": 2,
"size": 2,
},
]
`;
exports[`cli-explicit-extend/test.html 1`] = `
Array [
Object {
"column": 8,
"context": undefined,
"line": 1,
"message": "Mismatched close-tag, expected '</p>' but found '</i>'.",
"offset": 7,
"ruleId": "close-order",
"selector": null,
"severity": 2,
"size": 2,
},
]
`;
exports[`cli-no-extend/test.html 1`] = `Array []`;
{
"extends": ["html-validate:recommended"]
}
import path from "path";
import glob from "glob";
import { CLI } from "../src/cli/cli";
const fixtures = glob
.sync(path.join(__dirname, "cli-*/*.html"))
.map((filePath: string) => path.relative(__dirname, filePath));
it.each(fixtures)("%s", (filePath) => {
expect.assertions(1);
const cli = new CLI();
const htmlvalidate = cli.getValidator();
const report = htmlvalidate.validateFile(path.join(__dirname, filePath));
const result = report.results[0];
expect(result ? result.messages : []).toMatchSnapshot();
});
{
"root": true,
"extends": ["html-validate:recommended"],
"rules": {
"void": "off",
"void-style": ["error", { "style": "selfclose" }]
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment