Issue#16 comments are dropped (#17)

* Update rules to only do before To keep the rules simpler, and single responsibility they are now: - padding-before-test-block - padding-before-describe-block This is instead of putting the padding around them. It also now considers a comment to be padding * Update README and package.json
dangreenisrael · Apr 7, 2019 · f91369e · f91369e
1 parent d5ea0de
commit f91369e
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 132 deletions.
diff --git a/README.md b/README.md
@@ -1,5 +1,7 @@
 [![CircleCI](https://circleci.com/gh/dangreenisrael/eslint-plugin-jest-formatting/tree/master.svg?style=svg)](https://circleci.com/gh/dangreenisrael/eslint-plugin-jest-formatting/tree/master)
 
+# This is not stable yet, API will likely change before v1.0.0
+
 # eslint-plugin-jest-formatting
 
 Formatting rules for tests written in jest
@@ -38,8 +40,8 @@ Then configure the rules you want to use under the rules section.
 ```json
 {
     "rules": {
-        "jest-formatting/padding-test-blocks": 2,
-        "jest-formatting/padding-describe-blocks": 2,
+        "jest-formatting/padding-before-test-blocks": 2,
+        "jest-formatting/padding-before-describe-blocks": 2,
     }
 }
 ```

diff --git a/lib/rules/padding-describe-blocks.js → lib/rules/padding-before-describe-blocks.js b/lib/rules/padding-describe-blocks.js → lib/rules/padding-before-describe-blocks.js
@@ -4,15 +4,14 @@
  */
 "use strict";
 
-const { padBothSides } = require("../utils");
+const { padBefore } = require("../utils");
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
 
 const beforeMessage =
-  "You need a newline before a describe block when it comes after another expression";
-const afterMessage =
-  "You need a newline after a describe block when it comes before another expression";
+  "You need a newline or comment before a describe block when it comes after another expression";
+
 const isDescribe = node =>
   node.expression &&
   node.expression.callee &&
@@ -21,7 +20,7 @@ const isDescribe = node =>
 module.exports = {
   meta: {
     docs: {
-      description: "Enforces single line padding around describe blocks",
+      description: "Enforces at least a line of padding before describe blocks",
       category: "Fill me in",
       recommended: false
     },
@@ -31,7 +30,6 @@ module.exports = {
     ]
   },
   beforeMessage,
-  afterMessage,
   create(context) {
     const filePath = context && context.getFilename();
     const isTest =
@@ -43,7 +41,7 @@ module.exports = {
         if (!isTest || !isDescribe(node)) {
           return;
         }
-        padBothSides(context, node, isDescribe, beforeMessage, afterMessage);
+        padBefore({ context, node, beforeMessage });
       }
     };
   }

diff --git a/lib/rules/padding-test-blocks.js → lib/rules/padding-before-test-blocks.js b/lib/rules/padding-test-blocks.js → lib/rules/padding-before-test-blocks.js
@@ -4,15 +4,14 @@
  */
 "use strict";
 
-const { padBothSides } = require("../utils");
+const { padBefore } = require("../utils");
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
 
 const beforeMessage =
-  "You need a newline before an `it` or `test` block when it comes after another expression";
-const afterMessage =
-  "You need a newline after an `it` or `test` block when it comes after another expression";
+  "You need a newline or comment before an `it` or `test` block when it comes after another expression";
+
 const expressionName = node =>
   node.expression && node.expression.callee && node.expression.callee.name;
 const isTestBlock = node =>
@@ -22,7 +21,7 @@ module.exports = {
   meta: {
     docs: {
       description:
-        "Enforces a single line of padding between test blocks within a describe",
+        "Enforces at least a line of padding before test blocks within a describe",
       category: "Formatting",
       recommended: true
     },
@@ -32,7 +31,6 @@ module.exports = {
     ]
   },
   beforeMessage,
-  afterMessage,
   create(context) {
     const filePath = context && context.getFilename();
     const isTestFile =
@@ -44,7 +42,7 @@ module.exports = {
         if (!isTestFile || !isTestBlock(node)) {
           return;
         }
-        padBothSides(context, node, isTestBlock, beforeMessage, afterMessage);
+        padBefore({ context, node, beforeMessage });
       }
     };
   }

diff --git a/lib/utils.js b/lib/utils.js
@@ -2,20 +2,13 @@ const setPaddingBetweenNodes = ({
   context,
   problemNode,
   firstNode,
-  secondNode,
   message
 }) => {
   context.report({
     node: problemNode,
     message,
     fix: function(fixer) {
-      return [
-        fixer.removeRange([
-          firstNode.end,
-          secondNode.start - secondNode.loc.start.column
-        ]),
-        fixer.insertTextAfter(firstNode, "\n\n")
-      ];
+      return fixer.insertTextAfter(firstNode, "\n");
     }
   });
 };
@@ -26,60 +19,25 @@ const getLeftSibling = node => {
   return siblings[nodePosition - 1];
 };
 
-const getRightSibling = node => {
-  const siblings = node.parent.body;
-  const nodePosition = siblings.indexOf(node);
-  return siblings[nodePosition + 1];
-};
-
 const getStartLine = node => node && node.loc.start.line;
 
 const getEndLine = node => node && node.loc.end.line;
 
 const shouldFixGap = (bottomNode, topNode) =>
-  getStartLine(topNode) - getEndLine(bottomNode) !== 2;
+  getStartLine(topNode) - getEndLine(bottomNode) < 2;
 
-const padBefore = (context, node, qualifier, beforeMessage) => {
+const padBefore = ({ context, node, beforeMessage }) => {
   const leftSibling = getLeftSibling(node);
-  if (
-    leftSibling &&
-    !qualifier(leftSibling) &&
-    shouldFixGap(leftSibling, node)
-  ) {
+  if (leftSibling && shouldFixGap(leftSibling, node)) {
     setPaddingBetweenNodes({
       context,
       problemNode: node,
       firstNode: leftSibling,
-      secondNode: node,
       message: beforeMessage
     });
   }
 };
 
-const padAfter = (context, node, afterMessage) => {
-  const rightSibling = getRightSibling(node);
-  if (rightSibling && shouldFixGap(node, rightSibling)) {
-    setPaddingBetweenNodes({
-      context,
-      problemNode: node,
-      firstNode: node,
-      secondNode: rightSibling,
-      message: afterMessage
-    });
-  }
-};
-
-const padBothSides = (
-  context,
-  node,
-  qualifier,
-  beforeMessage,
-  afterMessage
-) => {
-  padBefore(context, node, qualifier, beforeMessage);
-  padAfter(context, node, afterMessage);
-};
-
 module.exports = {
-  padBothSides
+  padBefore
 };
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-jest-formatting",
-  "version": "0.0.6",
+  "version": "0.0.7",
   "description": "Formatting rules for test written with jest",
   "keywords": [
     "eslint",

diff --git a/...lib/rules/padding-describe-blocks.spec.js → ...es/padding-before-describe-blocks.spec.js b/...lib/rules/padding-describe-blocks.spec.js → ...es/padding-before-describe-blocks.spec.js
@@ -8,7 +8,7 @@
 // Requirements
 //------------------------------------------------------------------------------
 
-const rule = require("../../../lib/rules/padding-describe-blocks");
+const rule = require("../../../lib/rules/padding-before-describe-blocks");
 const RuleTester = require("eslint").RuleTester;
 
 RuleTester.setDefaultConfig({
@@ -21,53 +21,53 @@ RuleTester.setDefaultConfig({
 //------------------------------------------------------------------------------
 
 const validTopLevel = `
-describe('foo',()=>{});
-
 foo();
 bar();
 const thing="ok";
 
-describe('bar',()=>{});
+describe('bar',()=>{
 
-describe('baz',()=>{});
+});
 
-baz();
+describe('baz',()=>{
+
+});
 `;
 
 const invalidTopLevel = `
-describe('foo',()=>{});
 foo();
 bar();
 const thing="ok";
-describe('bar',()=>{});
-describe('baz',()=>{});
-baz();
+describe('bar',()=>{
+
+});
+describe('baz',()=>{
+
+});
 `;
 
 const validBlockLevel = `{
-describe('foo',()=>{});
+  foo();
+  bar();
 
-foo();
-bar();
+  describe('bar',()=>{
 
-describe('bar',()=>{
+  });
 
-});
+  describe('baz',()=>{
 
-describe('baz',()=>{});
-
-baz();
+  });
 }`;
 
 const invalidBlockLevel = `{
-describe('foo',()=>{});
-foo();
-bar();
-describe('bar',()=>{
+  foo();
+  bar();
+  describe('bar',()=>{
 
-});
-describe('baz',()=>{});
-baz();
+  });
+  describe('baz',()=>{
+
+  });
 }`;
 
 const ruleTester = new RuleTester();
@@ -78,20 +78,13 @@ ruleTester.run("padding-describe-blocks", rule, {
       code: invalidTopLevel,
       output: validTopLevel,
       errors: [
-        {
-          message: rule.afterMessage,
-          type: "ExpressionStatement"
-        },
         {
           message: rule.beforeMessage,
           type: "ExpressionStatement"
         },
+
         {
-          message: rule.afterMessage,
-          type: "ExpressionStatement"
-        },
-        {
-          message: rule.afterMessage,
+          message: rule.beforeMessage,
           type: "ExpressionStatement"
         }
       ]
@@ -100,20 +93,13 @@ ruleTester.run("padding-describe-blocks", rule, {
       code: invalidBlockLevel,
       output: validBlockLevel,
       errors: [
-        {
-          message: rule.afterMessage,
-          type: "ExpressionStatement"
-        },
         {
           message: rule.beforeMessage,
           type: "ExpressionStatement"
         },
+
         {
-          message: rule.afterMessage,
-          type: "ExpressionStatement"
-        },
-        {
-          message: rule.afterMessage,
+          message: rule.beforeMessage,
           type: "ExpressionStatement"
         }
       ]