Skip to content

Commit

Permalink
Normalize rule config values (#169)
Browse files Browse the repository at this point in the history
The rule validation script wasn't taking into account that extended configs can use numerical (`0, 1, 2`) or text (`off, warn, error`) notation for rule configs. This PR normalizes all rules in the snapshot to use text notation.

I had to parameterize this behavior to make an exception for when we compute the required Prettier rules, because Prettier intentionally uses numerical notation for a subset of its rules to distinguish them from the rest.

Applying this change caught some redundantly configured rules.
  • Loading branch information
rekmarks authored Apr 28, 2021
1 parent a2f8353 commit 8646767
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 42 deletions.
10 changes: 5 additions & 5 deletions packages/base/rules-snapshot.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"@typescript-eslint/no-extra-parens": "off",
"@typescript-eslint/no-extra-semi": "off",
"@typescript-eslint/object-curly-spacing": "off",
"@typescript-eslint/quotes": 0,
"@typescript-eslint/quotes": "off",
"@typescript-eslint/semi": "off",
"@typescript-eslint/space-before-function-paren": "off",
"@typescript-eslint/space-infix-ops": "off",
Expand All @@ -23,7 +23,7 @@
"arrow-parens": "off",
"arrow-spacing": "off",
"babel/object-curly-spacing": "off",
"babel/quotes": 0,
"babel/quotes": "off",
"babel/semi": "off",
"block-scoped-var": "error",
"block-spacing": "off",
Expand Down Expand Up @@ -128,7 +128,7 @@
"key-spacing": "off",
"keyword-spacing": "off",
"linebreak-style": "off",
"lines-around-comment": 0,
"lines-around-comment": "off",
"lines-between-class-members": "error",
"max-len": "off",
"max-statements-per-line": [
Expand Down Expand Up @@ -473,11 +473,11 @@
"vue/html-end-tags": "off",
"vue/html-indent": "off",
"vue/html-quotes": "off",
"vue/html-self-closing": 0,
"vue/html-self-closing": "off",
"vue/key-spacing": "off",
"vue/keyword-spacing": "off",
"vue/max-attributes-per-line": "off",
"vue/max-len": 0,
"vue/max-len": "off",
"vue/multiline-html-element-content-newline": "off",
"vue/mustache-interpolation-spacing": "off",
"vue/no-extra-parens": "off",
Expand Down
19 changes: 0 additions & 19 deletions packages/base/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,8 @@ module.exports = {
},
],

// Prettier has some opinions on mixed-operators, and there is ongoing work
// to make the output code clear. The workaround for keeping this rule enabled
// requires breaking parts of operations into different variables -- which we
// decided to be worse.
// https://github.com/prettier/eslint-config-prettier#no-mixed-operators
'no-mixed-operators': 'off',

// Prettier wraps e.g. single line functions with ternaries in parens by default, but
// if the line is long enough it breaks it into a separate line and removes the parens.
// The second behavior conflicts with this rule. There is some advice on the repo about
// how you can keep it enabled:
// https://github.com/prettier/eslint-config-prettier#no-confusing-arrow
// However, in practice this conflicts with prettier adding parens around short lines,
// when autofixing in vscode and others.
'no-confusing-arrow': 'off',

'curly': ['error', 'all'],
'max-len': 'off',
'no-tabs': 'error',
'no-unexpected-multiline': 'off',
'quotes': 'off',

// Not required by prettier, but potentially gotchas.
'no-restricted-syntax': ['error', 'SequenceExpression'],
Expand Down
68 changes: 50 additions & 18 deletions scripts/validate-configs.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,18 +360,17 @@ function getFlatConfigWithBaseConfig(configObject) {
* disabled when using Prettier.
*/
function getRequiredPrettierRules() {
return Object.entries(getFlatRules(getFlatConfig(prettierConfig))).reduce(
(allRules, [ruleName, ruleValue]) => {
// Rules set to 'off' should never be enabled.
// Rules set to 0 (number) may sometimes be included. We don't attend to those.
// https://github.com/Prettier/eslint-config-Prettier/blob/abf3ba1/index.js#L7-L9
if (ruleValue === OFF) {
allRules.push(ruleName);
}
return allRules;
},
[],
);
return Object.entries(
getFlatRules(getFlatConfig(prettierConfig), false),
).reduce((allRules, [ruleName, ruleValue]) => {
// Rules set to 'off' should never be enabled.
// Rules set to 0 (number) may sometimes be included. We don't attend to those.
// https://github.com/Prettier/eslint-config-Prettier/blob/abf3ba1/index.js#L7-L9
if (ruleValue === OFF) {
allRules.push(ruleName);
}
return allRules;
}, []);
}

//----------------
Expand All @@ -383,10 +382,13 @@ function getRequiredPrettierRules() {
* of its extended configs (if any) in a single, flat object.
*
* @param {Record<string, unknown>[]} flatConfig - A flat eslint config array.
* @param {boolean} [normalizeRules] - Whether to normalize rule config values
* to use string notation (off, warn, error) instead of numerical notation
* (0, 1, 2). Non-numerical values are passed through.
* @returns {Record<string, unknown>} An object of eslint rule names and their
* configuration.
*/
function getFlatRules(flatConfig) {
function getFlatRules(flatConfig, normalizeRules = true) {
// Flatten the config array into a single object
const rawFlatRules = flatConfig.reduce((flatRules, config) => {
if (RULES in config) {
Expand All @@ -399,25 +401,55 @@ function getFlatRules(flatConfig) {
}, {});

// Sort the flat rules alphabetically and return them
return sortObject(rawFlatRules);
return normalizeRules
? normalizeObject(rawFlatRules, normalizeRuleConfigValue)
: normalizeObject(rawFlatRules);
}

/**
* Sorts the keys of the given object, inserts them in that order in a new
* object, and returns that object.
* object, and returns that object. Optionally normalizes the values of the
* object during sorting.
*
* @param {Record<string, unknown>} obj - The object to sort.
* @param {Function} [valueNormalizer] - A function that takes a value and
* returns a "normalized" version of it. The value of every key on the sorted
* object will be passed through this function, if present.
* @returns {Record<string, unknown>} The sorted object.
*/
function sortObject(obj) {
function normalizeObject(obj, valueNormalizer) {
return Object.keys(obj)
.sort()
.reduce((sortedObj, key) => {
sortedObj[key] = obj[key];
sortedObj[key] = valueNormalizer ? valueNormalizer(obj[key]) : obj[key];
return sortedObj;
}, {});
}

/**
* Given an ESLint rule config value, converts it from numerical (0, 1, 2) to
* string (off, warn, error) notation, or just returns the given value.
*
* @param {unknown} configValue - The rule config value to normalize.
* @returns {string | typeof configValue} The normalized rule config value.
*/
function normalizeRuleConfigValue(configValue) {
if (typeof configValue !== 'number' && typeof configValue !== 'string') {
return configValue;
}

switch (String(configValue)) {
case '0':
return 'off';
case '1':
return 'warn';
case '2':
return 'error';
default:
return configValue;
}
}

/**
* Takes an eslint config object and flattens it and the configs it
* extends into a single array, ordered by their dependency relationships.
Expand Down Expand Up @@ -520,7 +552,7 @@ function logSnapshotViolations(snapshotViolations) {
console.error(
`\nError: Computed snapshot differs from the existing snapshot for the following package(s). Take a new snapshot and try again.\n\n${tabs(
1,
)}${snapshotViolations.join(`\n${tabs(1)}`)}`,
)}${snapshotViolations.join(`\n${tabs(1)}`)}\n`,
);
}

Expand Down

0 comments on commit 8646767

Please sign in to comment.