From ea987a30139b80d95548916c8f10fe48800354ad Mon Sep 17 00:00:00 2001 From: Sophie Kirschner Date: Mon, 1 Oct 2018 16:21:15 +0300 Subject: [PATCH 1/2] Update: "off" options for "space-before-blocks" (refs #10906) See https://github.com/eslint/eslint/issues/10906 Where "always" enforces space and "never" enforces no-space, "off" does not enforce any space preference. It may be useful to add a similar "off" option to other enums only allowing "always" and "never", too, but that is outside the scope of this PR. TODO: Why do two tests fail? (what the heck???) TODO: The documentation will need to describe this addition in more detail. --- docs/rules/space-before-blocks.md | 2 +- lib/rules/space-before-blocks.js | 79 ++++++++------- tests/lib/rules/space-before-blocks.js | 130 +++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 36 deletions(-) diff --git a/docs/rules/space-before-blocks.md b/docs/rules/space-before-blocks.md index 60f0717b3e1..ab4c2a2b400 100644 --- a/docs/rules/space-before-blocks.md +++ b/docs/rules/space-before-blocks.md @@ -17,7 +17,7 @@ This rule will enforce consistency of spacing before blocks. It is only applied This rule takes one argument. If it is `"always"` then blocks must always have at least one preceding space. If `"never"` then all blocks should never have any preceding space. If different spacing is desired for function blocks, keyword blocks and classes, an optional configuration object can be passed as the rule argument to -configure the cases separately. +configure the cases separately. If any value in the configuration object is `"off"`, then neither style will be enforced for blocks of that kind. ( e.g. `{ "functions": "never", "keywords": "always", "classes": "always" }` ) diff --git a/lib/rules/space-before-blocks.js b/lib/rules/space-before-blocks.js index 4f22ae6b653..9034c61db8b 100644 --- a/lib/rules/space-before-blocks.js +++ b/lib/rules/space-before-blocks.js @@ -32,13 +32,13 @@ module.exports = { type: "object", properties: { keywords: { - enum: ["always", "never"] + enum: ["always", "never", "off"] }, functions: { - enum: ["always", "never"] + enum: ["always", "never", "off"] }, classes: { - enum: ["always", "never"] + enum: ["always", "never", "off"] } }, additionalProperties: false @@ -51,18 +51,27 @@ module.exports = { create(context) { const config = context.options[0], sourceCode = context.getSourceCode(); - let checkFunctions = true, - checkKeywords = true, - checkClasses = true; + let alwaysFunctions = true, + alwaysKeywords = true, + alwaysClasses = true, + neverFunctions = false, + neverKeywords = false, + neverClasses = false; if (typeof config === "object") { - checkFunctions = config.functions !== "never"; - checkKeywords = config.keywords !== "never"; - checkClasses = config.classes !== "never"; + alwaysFunctions = config.functions === "always"; + alwaysKeywords = config.keywords === "always"; + alwaysClasses = config.classes === "always"; + neverFunctions = config.functions === "never"; + neverKeywords = config.keywords === "never"; + neverClasses = config.classes === "never"; } else if (config === "never") { - checkFunctions = false; - checkKeywords = false; - checkClasses = false; + alwaysFunctions = false; + alwaysKeywords = false; + alwaysClasses = false; + neverFunctions = true; + neverKeywords = true; + neverClasses = true; } /** @@ -88,35 +97,35 @@ module.exports = { const hasSpace = sourceCode.isSpaceBetweenTokens(precedingToken, node); const parent = context.getAncestors().pop(); let requireSpace; + let requireNoSpace; if (parent.type === "FunctionExpression" || parent.type === "FunctionDeclaration") { - requireSpace = checkFunctions; + requireSpace = alwaysFunctions; + requireNoSpace = neverFunctions; } else if (node.type === "ClassBody") { - requireSpace = checkClasses; + requireSpace = alwaysClasses; + requireNoSpace = neverClasses; } else { - requireSpace = checkKeywords; + requireSpace = alwaysKeywords; + requireNoSpace = neverKeywords; } - if (requireSpace) { - if (!hasSpace) { - context.report({ - node, - message: "Missing space before opening brace.", - fix(fixer) { - return fixer.insertTextBefore(node, " "); - } - }); - } - } else { - if (hasSpace) { - context.report({ - node, - message: "Unexpected space before opening brace.", - fix(fixer) { - return fixer.removeRange([precedingToken.range[1], node.range[0]]); - } - }); - } + if (requireSpace && !hasSpace) { + context.report({ + node, + message: "Missing space before opening brace.", + fix(fixer) { + return fixer.insertTextBefore(node, " "); + } + }); + } else if (requireNoSpace && hasSpace) { + context.report({ + node, + message: "Unexpected space before opening brace.", + fix(fixer) { + return fixer.removeRange([precedingToken.range[1], node.range[0]]); + } + }); } } } diff --git a/tests/lib/rules/space-before-blocks.js b/tests/lib/rules/space-before-blocks.js index e0ccb67ad0d..330ee60d4bd 100644 --- a/tests/lib/rules/space-before-blocks.js +++ b/tests/lib/rules/space-before-blocks.js @@ -21,6 +21,12 @@ const ruleTester = new RuleTester(), functionsOnlyArgs = [{ functions: "always", keywords: "never", classes: "never" }], keywordOnlyArgs = [{ functions: "never", keywords: "always", classes: "never" }], classesOnlyArgs = [{ functions: "never", keywords: "never", classes: "always" }], + functionsAlwaysOthersOffArgs = [{ functions: "always", keywords: "off", classes: "off" }], + keywordAlwaysOthersOffArgs = [{ functions: "off", keywords: "always", classes: "off" }], + classesAlwaysOthersOffArgs = [{ functions: "off", keywords: "off", classes: "always" }], + functionsNeverOthersOffArgs = [{ functions: "never", keywords: "off", classes: "off" }], + keywordNeverOthersOffArgs = [{ functions: "off", keywords: "never", classes: "off" }], + classesNeverOthersOffArgs = [{ functions: "off", keywords: "off", classes: "never" }], expectedSpacingErrorMessage = "Missing space before opening brace.", expectedSpacingError = { message: expectedSpacingErrorMessage }, expectedNoSpacingErrorMessage = "Unexpected space before opening brace.", @@ -131,6 +137,54 @@ ruleTester.run("space-before-blocks", rule, { code: "class test {}", parserOptions: { ecmaVersion: 6 } }, + { code: "function a(){if(b) {}}", options: keywordAlwaysOthersOffArgs }, + { code: "function a() {if(b) {}}", options: keywordAlwaysOthersOffArgs }, + { code: "function a() {if(b){}}", options: functionsAlwaysOthersOffArgs }, + { code: "function a() {if(b) {}}", options: functionsAlwaysOthersOffArgs }, + { + code: "class test { constructor(){if(a){}} }", + options: classesAlwaysOthersOffArgs, + parserOptions: { ecmaVersion: 6 } + }, + { + code: "class test { constructor() {if(a){}} }", + options: classesAlwaysOthersOffArgs, + parserOptions: { ecmaVersion: 6 } + }, + { + code: "class test { constructor(){if(a) {}} }", + options: classesAlwaysOthersOffArgs, + parserOptions: { ecmaVersion: 6 } + }, + { + code: "class test { constructor() {if(a) {}} }", + options: classesAlwaysOthersOffArgs, + parserOptions: { ecmaVersion: 6 } + }, + { code: "function a(){if(b){}}", options: keywordNeverOthersOffArgs }, + { code: "function a() {if(b){}}", options: keywordNeverOthersOffArgs }, + { code: "function a(){if(b){}}", options: functionsNeverOthersOffArgs }, + { code: "function a(){if(b) {}}", options: functionsNeverOthersOffArgs }, + { + code: "class test{ constructor(){if(a){}} }", + options: classesNeverOthersOffArgs, + parserOptions: { ecmaVersion: 6 } + }, + { + code: "class test{ constructor() {if(a){}} }", + options: classesNeverOthersOffArgs, + parserOptions: { ecmaVersion: 6 } + }, + { + code: "class test{ constructor(){if(a) {}} }", + options: classesNeverOthersOffArgs, + parserOptions: { ecmaVersion: 6 } + }, + { + code: "class test{ constructor() {if(a) {}} }", + options: classesNeverOthersOffArgs, + parserOptions: { ecmaVersion: 6 } + }, // https://github.com/eslint/eslint/issues/3769 { code: "()=>{};", options: ["always"], parserOptions: { ecmaVersion: 6 } }, @@ -427,6 +481,82 @@ ruleTester.run("space-before-blocks", rule, { options: neverArgs, parserOptions: { ecmaVersion: 6 }, errors: [expectedNoSpacingError] + }, + { + code: "if(a){ function a(){} }", + output: "if(a){ function a() {} }", + options: functionsAlwaysOthersOffArgs, + errors: [expectedSpacingError] + }, + { + code: "if(a) { function a(){} }", + output: "if(a) { function a() {} }", + options: functionsAlwaysOthersOffArgs, + errors: [expectedSpacingError] + }, + { + code: "if(a){ function a(){} }", + output: "if(a) { function a(){} }", + options: keywordAlwaysOthersOffArgs, + errors: [expectedSpacingError] + }, + { + code: "if(a){ function a() {} }", + output: "if(a) { function a() {} }", + options: keywordAlwaysOthersOffArgs, + errors: [expectedSpacingError] + }, + { + code: "class test{ constructor(){} }", + output: "class test { constructor(){} }", + options: classesAlwaysOthersOffArgs, + parserOptions: { ecmaVersion: 6 }, + errors: [expectedSpacingError] + }, + { + code: "class test{ constructor() {} }", + output: "class test { constructor() {} }", + options: classesAlwaysOthersOffArgs, + parserOptions: { ecmaVersion: 6 }, + errors: [expectedSpacingError] + }, + { + code: "if(a){ function a() {} }", + output: "if(a){ function a(){} }", + options: functionsNeverOthersOffArgs, + errors: [expectedNoSpacingError] + }, + { + code: "if(a) { function a() {} }", + output: "if(a) { function a(){} }", + options: functionsNeverOthersOffArgs, + errors: [expectedNoSpacingError] + }, + { + code: "if(a) { function a(){} }", + output: "if(a){ function a(){} }", + options: keywordNeverOthersOffArgs, + errors: [expectedNoSpacingError] + }, + { + code: "if(a) { function a() {} }", + output: "if(a){ function a() {} }", + options: keywordNeverOthersOffArgs, + errors: [expectedNoSpacingError] + }, + { + code: "class test { constructor(){} }", + output: "class test{ constructor(){} }", + options: classesNeverOthersOffArgs, + parserOptions: { ecmaNoVersion: 6 }, + errors: [expectedSpacingError] + }, + { + code: "class test { constructor() {} }", + output: "class test{ constructor() {} }", + options: classesNeverOthersOffArgs, + parserOptions: { ecmaNoVersion: 6 }, + errors: [expectedSpacingError] } ] }); From d130db86a8a387dacda4b1351f48327772851192 Mon Sep 17 00:00:00 2001 From: Sophie Kirschner Date: Tue, 2 Oct 2018 10:55:19 +0300 Subject: [PATCH 2/2] Fix: Tests pass for space-before-blocks rule change (refs #10907) I somehow managed to insert "No" into totally the wrong place in that last commit... --- tests/lib/rules/space-before-blocks.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/lib/rules/space-before-blocks.js b/tests/lib/rules/space-before-blocks.js index 330ee60d4bd..9b33f03a72b 100644 --- a/tests/lib/rules/space-before-blocks.js +++ b/tests/lib/rules/space-before-blocks.js @@ -548,15 +548,15 @@ ruleTester.run("space-before-blocks", rule, { code: "class test { constructor(){} }", output: "class test{ constructor(){} }", options: classesNeverOthersOffArgs, - parserOptions: { ecmaNoVersion: 6 }, - errors: [expectedSpacingError] + parserOptions: { ecmaVersion: 6 }, + errors: [expectedNoSpacingError] }, { code: "class test { constructor() {} }", output: "class test{ constructor() {} }", options: classesNeverOthersOffArgs, - parserOptions: { ecmaNoVersion: 6 }, - errors: [expectedSpacingError] + parserOptions: { ecmaVersion: 6 }, + errors: [expectedNoSpacingError] } ] });