New rules for checking links are valid

appmattus · Apr 17, 2019 · ee131b3 · ee131b3
1 parent 97f7782
commit ee131b3
Show file tree

Hide file tree

Showing 16 changed files with 181 additions and 24 deletions.
diff --git a/markdown/build.gradle.kts b/markdown/build.gradle.kts
@@ -53,6 +53,7 @@ dependencies {
     testImplementation("org.mockito:mockito-core:2.27.0")
     testImplementation("com.nhaarman.mockitokotlin2:mockito-kotlin:2.1.0")
     testImplementation("com.flextrade.jfixture:jfixture:2.7.2")
+    testImplementation("io.github.classgraph:classgraph:4.8.24")
 
     testImplementation("org.spekframework.spek2:spek-dsl-jvm:2.0.2")
     testRuntimeOnly("org.spekframework.spek2:spek-runner-junit5:2.0.2")

diff --git a/markdown/src/main/kotlin/com/appmattus/markdown/processing/MarkdownDocument.kt b/markdown/src/main/kotlin/com/appmattus/markdown/processing/MarkdownDocument.kt
@@ -35,9 +35,12 @@ import com.vladsch.flexmark.util.ast.NodeVisitor
 import com.vladsch.flexmark.util.ast.VisitHandler
 import com.vladsch.flexmark.util.sequence.BasedSequence
 import getLineNumberFixed
+import java.io.File
 import kotlin.reflect.KClass
 
-class MarkdownDocument(val filename: String, val document: Document) {
+class MarkdownDocument constructor(val file: File, val document: Document) {
+
+    val filename: String = file.name
 
     val headings: List<Heading> by lazy { document.find(Heading::class) }
     val htmlElements: List<Node>  by lazy { document.find(HtmlBlock::class, HtmlInline::class) }

diff --git a/markdown/src/main/kotlin/com/appmattus/markdown/processing/RuleProcessor.kt b/markdown/src/main/kotlin/com/appmattus/markdown/processing/RuleProcessor.kt
@@ -105,10 +105,8 @@ class RuleProcessor(private val rootDir: File, private val reportsDir: File) {
         val parser = ParserFactory.parser
 
         return associateWith { file ->
-            val document = MarkdownDocument(
-                file.name,
-                parser.parse(file.readText(Charsets.UTF_8))
-            )
+            val document = MarkdownDocument(file, parser.parse(file.readText(Charsets.UTF_8)))
+
             rules.flatMap { rule ->
                 val includes = MultiPathFilter(rule.configuration.includes, rootDir.toPath())
                 val excludes = MultiPathFilter(rule.configuration.excludes, rootDir.toPath())

diff --git a/markdown/src/main/kotlin/com/appmattus/markdown/rules/AllRules.kt b/markdown/src/main/kotlin/com/appmattus/markdown/rules/AllRules.kt
@@ -9,7 +9,9 @@ class AllRules(private val config: MarkdownLintConfig) {
         BlanksAroundListsRule(),
         CodeBlockStyleRule(),
         CommandsShowOutputRule(),
+        ConsistentEmphasisStyleRule(),
         ConsistentHeaderStyleRule(),
+        ConsistentTaskListMarkerStyleRule(),
         ConsistentUlStyleRule(),
         FencedCodeLanguageRule(),
         FirstHeaderH1Rule(),
@@ -21,6 +23,7 @@ class AllRules(private val config: MarkdownLintConfig) {
         ListIndentRule(),
         ListMarkerSpaceRule(),
         LowerCaseFilenameRule(),
+        MissingLinkSchemeRule(),
         NoBareUrlsRule(),
         NoBlanksBlockquoteRule(),
         NoConsecutiveHyphensFilenameRule(),
@@ -34,7 +37,7 @@ class AllRules(private val config: MarkdownLintConfig) {
         NoMultipleBlanksRule(),
         NoMultipleSpaceAtxRule(),
         NoMultipleSpaceBlockquoteRule(),
-        NoMissingSpaceClosedAtxRule(),
+        NoMultipleSpaceClosedAtxRule(),
         NoPunctuationFilenameRule(),
         NoReversedLinksRule(),
         NoSpaceInCodeRule(),
@@ -47,8 +50,10 @@ class AllRules(private val config: MarkdownLintConfig) {
         OlPrefixRule(),
         ProperNamesRule(),
         SingleH1Rule(),
+        TaskListMarkerSpaceRule(),
         UlIndentRule(),
-        UlStartLeftRule()
+        UlStartLeftRule(),
+        ValidRelativeLinksRule()
     )
 
     val rules: List<Rule> by lazy {

diff --git a/markdown/src/main/kotlin/com/appmattus/markdown/rules/MissingLinkSchemeRule.kt b/markdown/src/main/kotlin/com/appmattus/markdown/rules/MissingLinkSchemeRule.kt
@@ -0,0 +1,44 @@
+package com.appmattus.markdown.rules
+
+import com.appmattus.markdown.dsl.RuleSetup
+import com.appmattus.markdown.errors.ErrorReporter
+import com.appmattus.markdown.processing.MarkdownDocument
+import com.appmattus.markdown.rules.extentions.referenceUrl
+import com.vladsch.flexmark.ast.LinkRef
+import com.vladsch.flexmark.ast.Reference
+import java.net.URI
+
+/**
+ * # Missing link scheme
+ *
+ * This rule is triggered when a link is missing the scheme:
+ *
+ *     [missing scheme link](www.example.com)
+ *
+ * To fix the violation, provide a scheme for the link:
+ *
+ *     [a valid link](https://example.com/)
+ */
+class MissingLinkSchemeRule(
+    override val config: RuleSetup.Builder.() -> Unit = {}
+) : Rule() {
+
+    override fun visitDocument(document: MarkdownDocument, errorReporter: ErrorReporter) {
+        document.allLinks.forEach { link ->
+
+            when (link) {
+                is LinkRef -> link.referenceUrl()
+                is Reference -> null
+                else -> link.url
+            }?.let { url ->
+                val uri = URI(url.toString())
+
+                if (uri.scheme.isNullOrBlank() && uri.path.startsWith("www.")) {
+                    val description = "Link is missing http/https scheme, for example 'https://${link.chars}'"
+
+                    errorReporter.reportError(url.startOffset, url.endOffset, description)
+                }
+            }
+        }
+    }
+}
diff --git a/markdown/src/main/kotlin/com/appmattus/markdown/rules/ValidRelativeLinksRule.kt b/markdown/src/main/kotlin/com/appmattus/markdown/rules/ValidRelativeLinksRule.kt
@@ -0,0 +1,47 @@
+package com.appmattus.markdown.rules
+
+import com.appmattus.markdown.dsl.RuleSetup
+import com.appmattus.markdown.errors.ErrorReporter
+import com.appmattus.markdown.processing.MarkdownDocument
+import com.appmattus.markdown.rules.extentions.referenceUrl
+import com.vladsch.flexmark.ast.LinkRef
+import com.vladsch.flexmark.ast.Reference
+import java.io.File
+import java.net.URI
+
+/**
+ * # Relative links exist
+ *
+ * This rule is triggered when a relative link cannot be resolved:
+ *
+ *     [a relative link](this-file-does-not-exist.md)
+ *
+ * To fix the violation, ensure the linked file exists.
+ */
+class ValidRelativeLinksRule(
+    override val config: RuleSetup.Builder.() -> Unit = {}
+) : Rule() {
+
+    override fun visitDocument(document: MarkdownDocument, errorReporter: ErrorReporter) {
+        val parentDir = document.file.parent
+        document.allLinks.forEach { link ->
+            when (link) {
+                is LinkRef -> link.referenceUrl()
+                is Reference -> null
+                else -> link.url
+            }?.let { url ->
+                val uri = URI(url.toString())
+
+                if (uri.isRelative && uri.path.isNotEmpty() && !File(parentDir, uri.path).exists()) {
+                    val expected = File(document.file.parent, uri.path).normalize().path
+                    val description =
+                        "Relative link does not exist, '${link.chars}', expected at '$expected'"
+                    errorReporter.reportError(url.startOffset, url.endOffset, description)
+                }
+            }
+        }
+    }
+
+    private val URI.isRelative
+        get() = !isAbsolute && !path.startsWith("www.") && !path.startsWith("/")
+}
diff --git a/markdown/src/test/kotlin/com/appmattus/markdown/MarkdownDocumentExt.kt b/markdown/src/test/kotlin/com/appmattus/markdown/MarkdownDocumentExt.kt
@@ -2,12 +2,13 @@ package com.appmattus.markdown
 
 import com.appmattus.markdown.processing.MarkdownDocument
 import com.appmattus.markdown.processing.ParserFactory
+import java.io.File
 
 private val eolRegex = "(\\r?\\n|\\n)".toRegex()
 
 fun loadDocumentUnixEol(filename: String) =
     MarkdownDocument(
-        filename,
+        File(MarkdownDocument::class.java.classLoader.getResource(filename).file),
         ParserFactory.parser.parse(
             MarkdownDocument::class.java.classLoader.getResource(
                 filename
@@ -17,7 +18,7 @@ fun loadDocumentUnixEol(filename: String) =
 
 fun loadDocumentWindowsEol(filename: String) =
     MarkdownDocument(
-        filename,
+        File(MarkdownDocument::class.java.classLoader.getResource(filename).file),
         ParserFactory.parser.parse(
             MarkdownDocument::class.java.classLoader.getResource(
                 filename

diff --git a/markdown/src/test/kotlin/com/appmattus/markdown/rules/AllRulesTest.kt b/markdown/src/test/kotlin/com/appmattus/markdown/rules/AllRulesTest.kt
@@ -3,14 +3,18 @@ package com.appmattus.markdown.rules
 import com.appmattus.markdown.dsl.MarkdownLintConfig
 import com.appmattus.markdown.dsl.markdownLintConfig
 import com.appmattus.markdown.processing.MarkdownDocument
+import io.github.classgraph.ClassGraph
 import mockDocument
 import org.assertj.core.api.Assertions.assertThat
 import org.spekframework.spek2.Spek
 import org.spekframework.spek2.style.gherkin.Feature
+import java.io.File
 
 object AllRulesTest : Spek({
     Feature("AllRules") {
         val mockDocument = mockDocument()
+        val scanResult = ClassGraph().enableAllInfo().whitelistPackages("com.appmattus.markdown").scan()
+        val allExpectedRules = scanResult.getSubclasses(Rule::class.java.name).loadClasses()
 
         Scenario("empty configuration returns all rules") {
             lateinit var config: MarkdownLintConfig
@@ -27,7 +31,14 @@ object AllRulesTest : Spek({
             }
 
             Then("all rules are returned") {
-                assertThat(rules).size().isEqualTo(45)
+                val missing = allExpectedRules.filter { clazz ->
+                    rules.find { rule -> rule::class.java == clazz } == null
+                }.map {
+                    it.simpleName
+                }.toString()
+
+                assertThat(rules.size).`as`("Missing $missing")
+                    .isEqualTo(allExpectedRules.size)
             }
         }
 
@@ -50,7 +61,7 @@ object AllRulesTest : Spek({
             }
 
             Then("one less rule is returned") {
-                assertThat(rules).size().isEqualTo(44)
+                assertThat(rules.size).isEqualTo(allExpectedRules.size - 1)
             }
 
             And("all rules are active") {
@@ -75,24 +86,24 @@ object AllRulesTest : Spek({
             }
 
             Then("all rules are returned") {
-                assertThat(rules).size().isEqualTo(45)
+                assertThat(rules.size).isEqualTo(allExpectedRules.size)
             }
 
             And("the specified rule does not trigger as expected") {
                 val rule = rules.filterIsInstance(NoPunctuationFilenameRule::class.java).first()
-                val errors = rule.processDocument(MarkdownDocument("Z", mockDocument))
+                val errors = rule.processDocument(MarkdownDocument(File("Z"), mockDocument))
                 assertThat(errors).size().isZero
             }
 
             And("the specified rule triggers as expected") {
                 val rule = rules.filterIsInstance(NoPunctuationFilenameRule::class.java).first()
-                val errors = rule.processDocument(MarkdownDocument("A", mockDocument))
+                val errors = rule.processDocument(MarkdownDocument(File("A"), mockDocument))
                 assertThat(errors).size().isOne
             }
 
             And("the specified rule result differs to default config") {
                 val rule = NoPunctuationFilenameRule()
-                val errors = rule.processDocument(MarkdownDocument("A", mockDocument))
+                val errors = rule.processDocument(MarkdownDocument(File("A"), mockDocument))
                 assertThat(errors).size().isZero
             }
         }

diff --git a/markdown/src/test/kotlin/com/appmattus/markdown/rules/FeatureBodyExt.kt b/markdown/src/test/kotlin/com/appmattus/markdown/rules/FeatureBodyExt.kt
@@ -7,6 +7,7 @@ import com.appmattus.markdown.processing.MarkdownDocument
 import mockDocument
 import org.assertj.core.api.Assertions.assertThat
 import org.spekframework.spek2.style.gherkin.FeatureBody
+import java.io.File
 import kotlin.test.fail
 
 fun FeatureBody.FileRuleScenario(
@@ -91,7 +92,7 @@ fun FeatureBody.FilenameScenario(description: String, errors: Int, rules: () ->
         lateinit var ruleErrors: List<Error>
 
         Given("a document with filename \"$_filename\"") {
-            document = MarkdownDocument(_filename, mockDocument)
+            document = MarkdownDocument(File(_filename), mockDocument)
         }
 
         When("we visit the document") {

diff --git a/markdown/src/test/kotlin/com/appmattus/markdown/rules/LowerCaseFilenameRuleTest.kt b/markdown/src/test/kotlin/com/appmattus/markdown/rules/LowerCaseFilenameRuleTest.kt
@@ -6,6 +6,7 @@ import mockDocument
 import org.assertj.core.api.Assertions.assertThat
 import org.spekframework.spek2.Spek
 import org.spekframework.spek2.style.gherkin.Feature
+import java.io.File
 import java.util.UUID
 
 object LowerCaseFilenameRuleTest : Spek({
@@ -20,7 +21,7 @@ object LowerCaseFilenameRuleTest : Spek({
 
             Given("a document with lowercase filename") {
                 val filename = UUID.randomUUID().toString().toLowerCase()
-                document = MarkdownDocument(filename, mockDocument)
+                document = MarkdownDocument(File(filename), mockDocument)
             }
 
             When("we visit the document") {
@@ -38,7 +39,7 @@ object LowerCaseFilenameRuleTest : Spek({
 
             Given("a document with uppercase filename") {
                 val filename = UUID.randomUUID().toString().toUpperCase()
-                document = MarkdownDocument(filename, mockDocument)
+                document = MarkdownDocument(File(filename), mockDocument)
             }
 
             When("we visit the document") {
@@ -55,7 +56,7 @@ object LowerCaseFilenameRuleTest : Spek({
             lateinit var ruleErrors: List<Error>
 
             Given("a document with empty filename") {
-                document = MarkdownDocument("", mockDocument)
+                document = MarkdownDocument(File(""), mockDocument)
             }
 
             When("we visit the document") {

diff --git a/markdown/src/test/kotlin/com/appmattus/markdown/rules/MissingLinkSchemeRuleTest.kt b/markdown/src/test/kotlin/com/appmattus/markdown/rules/MissingLinkSchemeRuleTest.kt
@@ -0,0 +1,12 @@
+package com.appmattus.markdown.rules
+
+import org.spekframework.spek2.Spek
+import org.spekframework.spek2.style.gherkin.Feature
+
+object MissingLinkSchemeRuleTest : Spek({
+    Feature("MissingLinkSchemeRule") {
+        FileRuleScenario(listOf("relative-links.md")) { MissingLinkSchemeRule() }
+
+        FileRuleScenario { MissingLinkSchemeRule() }
+    }
+})
diff --git a/markdown/src/test/kotlin/com/appmattus/markdown/rules/ValidRelativeLinksRuleTest.kt b/markdown/src/test/kotlin/com/appmattus/markdown/rules/ValidRelativeLinksRuleTest.kt
@@ -0,0 +1,12 @@
+package com.appmattus.markdown.rules
+
+import org.spekframework.spek2.Spek
+import org.spekframework.spek2.style.gherkin.Feature
+
+object ValidRelativeLinksRuleTest : Spek({
+    Feature("ValidRelativeLinksRule") {
+        FileRuleScenario(listOf("relative-links.md")) { ValidRelativeLinksRule() }
+
+        FileRuleScenario { ValidRelativeLinksRule() }
+    }
+})
diff --git a/markdown/src/test/resources/break-all-the-rules.md b/markdown/src/test/resources/break-all-the-rules.md
@@ -68,10 +68,12 @@ Emphasis *with * space {NoSpaceInEmphasisRule}
 
 Code `with ` space {NoSpaceInCodeRule}
 
-[link with space ](link) {NoSpaceInLinksRule}
+[link with space ](link) {NoSpaceInLinksRule} {ValidRelativeLinksRule}
+
+[link without scheme](www.example.com) {MissingLinkSchemeRule}
 
 ```
-code fence without language {FencedCodeLanguageRule:73}
+code fence without language {FencedCodeLanguageRule:75}
 ```
 
 [empty link]() {NoEmptyLinksRule}

diff --git a/markdown/src/test/resources/empty-links.md b/markdown/src/test/resources/empty-links.md
@@ -14,11 +14,11 @@
 
 ## Non-empty links
 
-[text](link)
+[text](link) {ValidRelativeLinksRule}
 
-[text](link "title")
+[text](link "title") {ValidRelativeLinksRule}
 
-[text](<link>)
+[text](<link>) {ValidRelativeLinksRule}
 
 [text](#frag)
 
@@ -29,3 +29,5 @@
 [text]
 
 [text]: link
+
+{ValidRelativeLinksRule:27} {ValidRelativeLinksRule:31}
diff --git a/markdown/src/test/resources/fix_102_extra_nodes_in_link_text.md b/markdown/src/test/resources/fix_102_extra_nodes_in_link_text.md
@@ -11,3 +11,7 @@
 {ConsistentEmphasisStyleRule:4} {ConsistentEmphasisStyleRule:5}
 {ConsistentEmphasisStyleRule:5} {ConsistentEmphasisStyleRule:5}
 {ConsistentEmphasisStyleRule:6}
+
+{MissingLinkSchemeRule:1} {MissingLinkSchemeRule:2} {MissingLinkSchemeRule:3}
+{MissingLinkSchemeRule:4} {MissingLinkSchemeRule:5} {MissingLinkSchemeRule:6}
+{MissingLinkSchemeRule:7} {MissingLinkSchemeRule:8}
diff --git a/markdown/src/test/resources/relative-links.md b/markdown/src/test/resources/relative-links.md
@@ -0,0 +1,13 @@
+# Relative links
+
+[This exists](relative-links.md)
+
+[This also exists](./relative-links.md)
+
+[This does not exist](./nothing-to-see-here.md) {ValidRelativeLinksRule}
+
+[This is absolute](/so-should-not-trigger.md)
+
+[This is a web url](www.example.com) {MissingLinkSchemeRule}
+
+[This is a web url](https://www.example.com)