Skip to content

Commit

Permalink
Use default js-yaml schema (palantir#4350)
Browse files Browse the repository at this point in the history
This will change yaml parser to use the default schema, which allows
some additional Yaml types, see http://yaml.org/type/

Namely, this will allow the merge syntax, allowing users to define
common rules and then merge them into the typescript and javascript
rules.

Note: To do this, the options passed to yaml.safeLoad have been
removed since they are no longer required. The default safe schema
is what we want, and the strict: true is not a valid option, since
js-yaml removed it a while back, see here:
nodeca/js-yaml@e6bac15
  • Loading branch information
mogzol authored and Josh Goldberg committed Dec 7, 2018
1 parent a77ddd4 commit 694e59a
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 7 deletions.
8 changes: 1 addition & 7 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,7 @@ export function readConfigurationFile(filepath: string): RawConfigFile {
if (resolvedConfigFileExt === ".json") {
return JSON.parse(stripComments(fileContent)) as RawConfigFile;
} else {
return yaml.safeLoad(fileContent, {
// Note: yaml.LoadOptions expects a schema value of type "any",
// but this trips up the no-unsafe-any rule.
// tslint:disable-next-line:no-unsafe-any
schema: yaml.JSON_SCHEMA,
strict: true,
}) as RawConfigFile;
return yaml.safeLoad(fileContent) as RawConfigFile;
}
} catch (e) {
const error = e as Error;
Expand Down
18 changes: 18 additions & 0 deletions test/config/tslint-with-merge.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
commonRules: &common
rule-one:
severity: error
rule-two:
- true
- "common rule"
jsRules:
<<: *common
rule-three:
- true
- "js rule"
rules:
<<: *common
rule-one:
severity: warning
rule-three:
- true
- "ts rule"
35 changes: 35 additions & 0 deletions test/configurationTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,41 @@ describe("Configuration", () => {
assertRulesEqual(config.jsRules, expectedRules);
});

it("can load .yaml files with merge key", () => {
const config = loadConfigurationFromPath("./test/config/tslint-with-merge.yaml");

const expectedRules = getEmptyRules();
expectedRules.set("rule-one", {
ruleArguments: undefined,
ruleSeverity: "warning",
});
expectedRules.set("rule-two", {
ruleArguments: ["common rule"],
ruleSeverity: "error",
});
expectedRules.set("rule-three", {
ruleArguments: ["ts rule"],
ruleSeverity: "error",
});

const expectedJsRules = getEmptyRules();
expectedJsRules.set("rule-one", {
ruleArguments: undefined,
ruleSeverity: "error",
});
expectedJsRules.set("rule-two", {
ruleArguments: ["common rule"],
ruleSeverity: "error",
});
expectedJsRules.set("rule-three", {
ruleArguments: ["js rule"],
ruleSeverity: "error",
});

assertRulesEqual(config.rules, expectedRules);
assertRulesEqual(config.jsRules, expectedJsRules);
});

it("can load a built-in configuration", () => {
const config = loadConfigurationFromPath("tslint:recommended");
assert.strictEqual<RuleSeverity | undefined>(
Expand Down

0 comments on commit 694e59a

Please sign in to comment.