fix(rule): fix handling of multi-line text (#4)

* fix(rule): fix handling of multi-line text Previously, the rule handle list's content as a single line text. To fix this, use and report per child node * chore(deps): remove unist-util-select * chore(deps): update textlint-scripts * chore: update comment * fix(rule): use ParagraphNode instead of node * chore(rule): move skip rule to op
textlint-rule · Jun 15, 2019 · 1452282 · 1452282
1 parent c331b43
commit 1452282
Show file tree

Hide file tree

Showing 4 changed files with 1,282 additions and 1,139 deletions.
diff --git a/package.json b/package.json
@@ -31,10 +31,9 @@
   },
   "homepage": "https://github.com/textlint-rule/textlint-rule-period-in-list-item",
   "devDependencies": {
-    "textlint-scripts": "^1.2.2"
+    "textlint-scripts": "^2.1.0"
   },
   "dependencies": {
-    "check-ends-with-period": "^1.0.1",
-    "unist-util-select": "^1.5.0"
+    "check-ends-with-period": "^1.0.1"
   }
-}
+}
diff --git a/src/textlint-rule-period-in-list-item.js b/src/textlint-rule-period-in-list-item.js
@@ -1,6 +1,5 @@
 // MIT © 2017 azu
 "use strict";
-const select = require('unist-util-select');
 const checkEndsWithPeriod = require("check-ends-with-period");
 /**
  * check `text` that the end is not periodMark
@@ -83,69 +82,76 @@ const reporter = (context, options = {}) => {
         : defaultOptions.forceAppendPeriod;
     return {
         [Syntax.ListItem](node) {
-            const text = getSource(node);
-            // Skip Ordered List item if option is enabled
-            if (allowOrderedList && isItemNodeInOrderedList(node)) {
+            // A ListItem should includes child nodes.
+            // https://github.com/textlint-rule/textlint-rule-period-in-list-item/issues/3
+            const PragraphNodes = node.children;
+            if (!Array.isArray(PragraphNodes)){
                 return;
             }
-            // Prefer no needed period, but exist period
-            if (isNotNeededPeriodMark) {
-                const {valid, periodMark, index} = checkEndsWithoutPeriodMark(text, periodMarks);
-                if (valid) {
-                    return;
-                }
-                // should be remove period mark
-                report(node, new RuleError(`Should remove period mark("${periodMark}") at end of list item.`, {
-                    index,
-                    fix: fixer.replaceTextRange([index, index + periodMark.length], "")
-                }));
+            // Skip Ordered List item if option is enabled
+            if (allowOrderedList && isItemNodeInOrderedList(node)) {
                 return;
             }
-            // - [link](http://example)
-            // should be ignored
-            if (ignoreLinkEnd) {
-                const linkNodes = select(node, `${Syntax.Paragraph} > *`);
-                if (linkNodes.length === 1 && linkNodes[0].type === Syntax.Link) {
-                    return;
-                }
-            }
-            const {valid, periodMark, index} = checkEndsWithPeriod(text, {
-                periodMarks,
-                allowPeriodMarks,
-                allowEmoji,
-            });
-            // Prefer to use period
-            if (valid) {
-                //  but exist difference period
-                const isPeriodMarkAtEnd = periodMarks.indexOf(periodMark) !== -1;
-                // exception case that should not report
-                // !?
-                if (!isPeriodMarkAtEnd) {
+            PragraphNodes.forEach(ParagraphNode => {
+                const text = getSource(ParagraphNode);
+                // Prefer no needed period, but exist period
+                if (isNotNeededPeriodMark) {
+                    const { valid, periodMark, index } = checkEndsWithoutPeriodMark(text, periodMarks);
+                    if (valid) {
+                        return;
+                    }
+                    // should be remove period mark
+                    report(ParagraphNode, new RuleError(`Should remove period mark("${periodMark}") at end of list item.`, {
+                        index,
+                        fix: fixer.replaceTextRange([index, index + periodMark.length], "")
+                    }));
                     return;
                 }
-                // periodMark is expected, then exit
-                if (periodMark === preferPeriodMark) {
-                    return;
+                // - [link](http://example)
+                // should be ignored
+                if (ignoreLinkEnd) {
+                    const linkNodes = ParagraphNode.children;
+                    if (linkNodes.length === 1 && linkNodes[0].type === Syntax.Link) {
+                        return;
+                    }
                 }
-                report(node, new RuleError(`Prefer to use period mark("${preferPeriodMark}") at end of list item.`, {
-                    index,
-                    fix: fixer.replaceTextRange([index, index + periodMark.length], preferPeriodMark)
-                }));
-                return;
-            } else {
-                // but not exist period
-                if (forceAppendPeriod) {
-                    report(node, new RuleError(`Not exist period mark("${preferPeriodMark}") at end of list item.`, {
+                const { valid, periodMark, index } = checkEndsWithPeriod(text, {
+                    periodMarks,
+                    allowPeriodMarks,
+                    allowEmoji,
+                });
+                // Prefer to use period
+                if (valid) {
+                    //  but exist difference period
+                    const isPeriodMarkAtEnd = periodMarks.indexOf(periodMark) !== -1;
+                    // exception case that should not report
+                    // !?
+                    if (!isPeriodMarkAtEnd) {
+                        return;
+                    }
+                    // periodMark is expected, then exit
+                    if (periodMark === preferPeriodMark) {
+                        return;
+                    }
+                    report(ParagraphNode, new RuleError(`Prefer to use period mark("${preferPeriodMark}") at end of list item.`, {
                         index,
-                        fix: fixer.replaceTextRange([index + 1, index + 1], preferPeriodMark)
+                        fix: fixer.replaceTextRange([index, index + periodMark.length], preferPeriodMark)
                     }));
+                    return;
                 } else {
-                    report(node, new RuleError(`Not exist period mark("${preferPeriodMark}") at end of list item.`, {
-                        index
-                    }))
+                    // but not exist period
+                    if (forceAppendPeriod) {
+                        report(ParagraphNode, new RuleError(`Not exist period mark("${preferPeriodMark}") at end of list item.`, {
+                            index,
+                            fix: fixer.replaceTextRange([index + 1, index + 1], preferPeriodMark)
+                        }));
+                    } else {
+                        report(ParagraphNode, new RuleError(`Not exist period mark("${preferPeriodMark}") at end of list item.`, {
+                            index
+                        }))
+                    }
                 }
-
-            }
+            })
         }
     }
 };

diff --git a/test/textlint-rule-period-in-list-item-test.js b/test/textlint-rule-period-in-list-item-test.js
@@ -71,7 +71,25 @@ tester.run("textlint-rule-period-in-list-item", rule, {
                 allowOrderedList: true
             }
         },
+        // https://github.com/textlint-rule/textlint-rule-period-in-list-item/issues/3
+        {
+            text: `
+- list item 1.
+
+    A paragraph describing this list item 1.
+
+- list item 2.
+
+    A paragraph describing this list item.
+
+- list item 3.
 
+    Another paragraph describing list item 3.
+`,
+            options: {
+                periodMark: "."
+            }
+        },
     ],
     invalid: [
         // remove period mark
@@ -182,6 +200,26 @@ tester.run("textlint-rule-period-in-list-item", rule, {
                     column: 8
                 }
             ]
-        }
+        },
+        // https://github.com/textlint-rule/textlint-rule-period-in-list-item/issues/3
+        {
+            text: `
+- list item 1
+
+    A paragraph describing this list item 1.
+
+- list item 2
+
+    A paragraph describing this list item.
+
+- list item 3
+
+    Another paragraph describing list item 3.
+`,
+            options: {
+                periodMark: "."
+            },
+            errors: [{ line: 2, column: 13 }, { line: 6, column: 13 }, { line: 10, column: 13 }]
+        },
     ]
 });