Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Normalize rule config values #169

Merged
merged 2 commits into from
Apr 28, 2021
Merged

Normalize rule config values #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
56e8df7
Normalize rule config values
rekmarks Apr 19, 2021
282096f
fixup! Normalize rule config values
rekmarks Apr 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/base/rules-snapshot.json
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, interesting way of denoting optional settings.

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