diff --git a/elm.json b/elm.json index 7c287491..f1508294 100644 --- a/elm.json +++ b/elm.json @@ -22,6 +22,7 @@ "stil4m/elm-syntax": "7.2.7 <= v < 8.0.0" }, "test-dependencies": { - "elm/regex": "1.0.0 <= v < 2.0.0" + "elm/regex": "1.0.0 <= v < 2.0.0", + "elm/parser": "1.1.0 <= v < 2.0.0" } } diff --git a/review/elm.json b/review/elm.json index 5c58e1c4..dd638cc0 100644 --- a/review/elm.json +++ b/review/elm.json @@ -9,6 +9,7 @@ "direct": { "elm/core": "1.0.5", "elm/json": "1.1.3", + "elm/parser": "1.1.0", "elm/project-metadata-utils": "1.0.2", "elm/regex": "1.0.0", "jfmengels/elm-review": "2.4.0", @@ -16,7 +17,6 @@ }, "indirect": { "elm/html": "1.0.0", - "elm/parser": "1.1.0", "elm/random": "1.0.0", "elm/time": "1.0.0", "elm/virtual-dom": "1.0.2", diff --git a/review/src/ReviewConfig.elm b/review/src/ReviewConfig.elm index dc8cf1aa..26698766 100644 --- a/review/src/ReviewConfig.elm +++ b/review/src/ReviewConfig.elm @@ -11,7 +11,7 @@ when inside the directory containing this file. -} -import Documentation.ReadmeLinksPointToCurrentVersion +import Docs.UpToDateReadmeLinks import NoDebug.Log import NoDebug.TodoOrToString import NoExposingEverything @@ -32,7 +32,7 @@ import Review.Rule as Rule exposing (Rule) config : List Rule config = - [ Documentation.ReadmeLinksPointToCurrentVersion.rule + [ Docs.UpToDateReadmeLinks.rule , NoDebug.Log.rule , NoDebug.TodoOrToString.rule |> Rule.ignoreErrorsForDirectories [ "tests/" ] diff --git a/tests/Docs/NoMissing.elm b/tests/Docs/NoMissing.elm new file mode 100644 index 00000000..39e9579b --- /dev/null +++ b/tests/Docs/NoMissing.elm @@ -0,0 +1,428 @@ +module Docs.NoMissing exposing + ( rule + , What, everything, onlyExposed + , From, allModules, exposedModules + ) + +{-| + +@docs rule + + +## Configuration + +@docs What, everything, onlyExposed +@docs From, allModules, exposedModules + + +## When (not) to enable this rule + +This rule is useful when you care about having a thoroughly or increasingly documented project. +It is also useful when you write Elm packages, in order to know about missing documentation before you publish. + + +## Try it out + +You can try this rule out by running the following command: + +```bash +elm-review --template jfmengels/elm-review-documentation/example --rules Docs.NoMissing +``` + +-} + +import Docs.Utils.ExposedFromProject as ExposedFromProject +import Elm.Project +import Elm.Syntax.Declaration as Declaration exposing (Declaration) +import Elm.Syntax.Exposing as Exposing +import Elm.Syntax.Module as Module exposing (Module) +import Elm.Syntax.Node as Node exposing (Node(..)) +import Elm.Syntax.Range as Range +import Review.Rule as Rule exposing (Error, Rule) +import Set exposing (Set) + + +{-| Reports missing or empty documentation for functions and types. + + import Docs.NoMissing exposing (exposedModules, onlyExposed) + + config = + [ Docs.NoMissing.rule + { document = onlyExposed + , from = exposedModules + } + ] + + +## Fail + +The rule will report when documentation is missing + + someFunction = + great Things + +or when documentation is present but empty. + + {-| -} + someOtherFunction = + other (great Things) + +The reasoning for not allowing empty documentation is because of how people consume documentation can vary, and for some +of those ways, empty documentation doesn't lead to a nice experience. For instance, if you are looking at Elm code in +your IDE and want to lookup the definition of a function, an empty documentation will give you no information beyond the +type annotation. + +When I write documentation for a module, I try to tell a story or somehow phrase it as a tutorial, so that people can +learn the easier concepts first, and gradually as they read more and learn more about the ideas and concepts, I will +assume that they read most of the documentation above. + +But for every function or type, I also imagine that they'll be read on their own from an IDE for instance, and therefore +try to make the documentation as light as possible while giving a helpful description and an example, without relying +too much on the assumption that the user has read the rest of the module. + +A common case where people don't give an example is when exposing functions such as `map2`, `map3`, `map4`, etc., usually +documented in that order and next to each other. While `map2` is usually properly documented, the following ones would +have empty documentation, which I believe would be because the author assumes that the user went through the documentation on +the package registry and has read the documentation for `map2`. But if someone unfamiliar with Elm or an API looks up +`map3`, they may have trouble finding the information they were looking for. + +I would recommend to make the documentation for each element as understandable out of context as possible. At the very least, +I would advise to say something like "This function is like `map2` but with X arguments" with a link to `map2`, so that +relevant information _can_ be found without too much effort. + + +## Success + + {-| someFunction does great things + -} + someFunction = + great Things + +-} +rule : { document : What, from : From } -> Rule +rule configuration = + Rule.newModuleRuleSchema "Docs.NoMissing" initialContext + |> Rule.withElmJsonModuleVisitor elmJsonVisitor + |> Rule.withModuleDefinitionVisitor (moduleDefinitionVisitor configuration.from) + |> Rule.withCommentsVisitor commentsVisitor + |> Rule.withDeclarationEnterVisitor (declarationVisitor configuration.document) + |> Rule.fromModuleRuleSchema + + +type alias Context = + { moduleNameNode : Node String + , exposedModules : Set String + , exposedElements : Exposed + , shouldBeReported : Bool + } + + +initialContext : Context +initialContext = + { moduleNameNode = Node Range.emptyRange "" + , exposedModules = Set.empty + , exposedElements = EverythingIsExposed + , shouldBeReported = True + } + + +type Exposed + = EverythingIsExposed + | ExplicitList (Set String) + + +{-| Which elements from a module should be documented. Possible options are [`everything`](#everything) in a module or +only the exposed elements of a module ([`onlyExposed`](#onlyExposed)). +-} +type What + = Everything + | OnlyExposed + + +{-| Every function and type from a module should be documented. The module definition should also be documented. +-} +everything : What +everything = + Everything + + +{-| Only exposed functions and types from a module should be documented. The module definition should also be documented. +-} +onlyExposed : What +onlyExposed = + OnlyExposed + + +{-| Which modules should be documented. Possible options are [`allModules`](#allModules) of a project or +only the [`exposedModules`](#exposedModules) (only for packages). +-} +type From + = AllModules + | ExposedModules + + +{-| All modules from the project should be documented. +-} +allModules : From +allModules = + AllModules + + +{-| Only exposed modules from the project will need to be documented. + +If your project is an application, you should not use this option. An application does not expose modules which would +mean there isn't any module to report errors for. + +-} +exposedModules : From +exposedModules = + -- TODO Report a global error if used inside an application + ExposedModules + + + +-- ELM.JSON VISITOR + + +elmJsonVisitor : Maybe Elm.Project.Project -> Context -> Context +elmJsonVisitor maybeProject context = + let + exposedModules_ : Set String + exposedModules_ = + case maybeProject of + Just project -> + ExposedFromProject.exposedModules project + + _ -> + Set.empty + in + { context | exposedModules = exposedModules_ } + + + +-- MODULE DEFINITION VISITOR + + +moduleDefinitionVisitor : From -> Node Module -> Context -> ( List nothing, Context ) +moduleDefinitionVisitor fromConfig node context = + let + moduleNameNode : Node String + moduleNameNode = + case Node.value node of + Module.NormalModule x -> + Node + (Node.range x.moduleName) + (Node.value x.moduleName |> String.join ".") + + Module.PortModule x -> + Node + (Node.range x.moduleName) + (Node.value x.moduleName |> String.join ".") + + Module.EffectModule x -> + Node + (Node.range x.moduleName) + (Node.value x.moduleName |> String.join ".") + + shouldBeReported : Bool + shouldBeReported = + case fromConfig of + AllModules -> + True + + ExposedModules -> + Set.member (Node.value moduleNameNode) context.exposedModules + + exposed : Exposed + exposed = + case Node.value node |> Module.exposingList of + Exposing.All _ -> + EverythingIsExposed + + Exposing.Explicit list -> + ExplicitList (List.map collectExposing list |> Set.fromList) + in + ( [] + , { context + | moduleNameNode = moduleNameNode + , shouldBeReported = shouldBeReported + , exposedElements = exposed + } + ) + + +collectExposing : Node Exposing.TopLevelExpose -> String +collectExposing node = + case Node.value node of + Exposing.InfixExpose name -> + name + + Exposing.FunctionExpose name -> + name + + Exposing.TypeOrAliasExpose name -> + name + + Exposing.TypeExpose exposedType -> + exposedType.name + + + +-- COMMENTS VISITOR + + +commentsVisitor : List (Node String) -> Context -> ( List (Error {}), Context ) +commentsVisitor comments context = + if context.shouldBeReported then + let + documentation : Maybe (Node String) + documentation = + findFirst (Node.value >> String.startsWith "{-|") comments + in + ( checkModuleDocumentation documentation context.moduleNameNode + , context + ) + + else + ( [], context ) + + +findFirst : (a -> Bool) -> List a -> Maybe a +findFirst predicate list = + case list of + [] -> + Nothing + + a :: rest -> + if predicate a then + Just a + + else + findFirst predicate rest + + + +-- DECLARATION VISITOR + + +declarationVisitor : What -> Node Declaration -> Context -> ( List (Error {}), Context ) +declarationVisitor documentWhat node context = + if context.shouldBeReported then + ( reportDeclarationDocumentation documentWhat context node + , context + ) + + else + ( [], context ) + + +reportDeclarationDocumentation : What -> Context -> Node Declaration -> List (Error {}) +reportDeclarationDocumentation documentWhat context node = + case Node.value node of + Declaration.FunctionDeclaration { documentation, declaration } -> + let + nameNode : Node String + nameNode = + (Node.value declaration).name + in + if shouldBeDocumented documentWhat context (Node.value nameNode) then + checkElementDocumentation documentation nameNode + + else + [] + + Declaration.CustomTypeDeclaration { documentation, name } -> + if shouldBeDocumented documentWhat context (Node.value name) then + checkElementDocumentation documentation name + + else + [] + + Declaration.AliasDeclaration { documentation, name } -> + if shouldBeDocumented documentWhat context (Node.value name) then + checkElementDocumentation documentation name + + else + [] + + _ -> + [] + + +shouldBeDocumented : What -> Context -> String -> Bool +shouldBeDocumented documentWhat context name = + case documentWhat of + Everything -> + True + + OnlyExposed -> + case context.exposedElements of + EverythingIsExposed -> + True + + ExplicitList exposedElements -> + Set.member name exposedElements + + +checkModuleDocumentation : Maybe (Node String) -> Node String -> List (Error {}) +checkModuleDocumentation documentation nameNode = + case documentation of + Just doc -> + if isDocumentationEmpty doc then + [ Rule.error + { message = "The documentation for module `" ++ Node.value nameNode ++ "` is empty" + , details = [ "Empty documentation is not useful for the users. Please give explanations or examples." ] + } + (Node.range doc) + ] + + else + [] + + Nothing -> + [ Rule.error + { message = "Missing documentation for module `" ++ Node.value nameNode ++ "`" + , details = documentationErrorDetails + } + (Node.range nameNode) + ] + + +documentationErrorDetails : List String +documentationErrorDetails = + [ "A module documentation summarizes what a module is for, the responsibilities it has and how to use it. Providing a good module documentation will be useful for your users or colleagues." + ] + + +checkElementDocumentation : Maybe (Node String) -> Node String -> List (Error {}) +checkElementDocumentation documentation nameNode = + case documentation of + Just doc -> + if isDocumentationEmpty doc then + [ Rule.error + { message = "The documentation for `" ++ Node.value nameNode ++ "` is empty" + , details = [ "Empty documentation is not useful for the users. Please give explanations or examples." ] + } + (Node.range doc) + ] + + else + [] + + Nothing -> + [ Rule.error + { message = "Missing documentation for `" ++ Node.value nameNode ++ "`" + , details = [ "Documentation can help developers use this API." ] + } + (Node.range nameNode) + ] + + +isDocumentationEmpty : Node String -> Bool +isDocumentationEmpty doc = + doc + |> Node.value + |> String.dropLeft 3 + |> String.dropRight 2 + |> String.trim + |> String.isEmpty diff --git a/tests/Docs/NoMissingTest.elm b/tests/Docs/NoMissingTest.elm new file mode 100644 index 00000000..f22de941 --- /dev/null +++ b/tests/Docs/NoMissingTest.elm @@ -0,0 +1,369 @@ +module Docs.NoMissingTest exposing (all) + +import Docs.NoMissing exposing (rule) +import Elm.Project +import Json.Decode as Decode +import Review.Project as Project exposing (Project) +import Review.Test +import Test exposing (Test, describe, test) + + +missingModuleDetails : List String +missingModuleDetails = + [ "A module documentation summarizes what a module is for, the responsibilities it has and how to use it. Providing a good module documentation will be useful for your users or colleagues." + ] + + +missingElementDetails : List String +missingElementDetails = + [ "Documentation can help developers use this API." ] + + +all : Test +all = + describe "Docs.NoMissing" + [ everythingEverywhereTests + , everythingFromExposedModulesTests + , onlyExposedFromExposedModulesTests + ] + + +everythingEverywhereTests : Test +everythingEverywhereTests = + let + config : { document : Docs.NoMissing.What, from : Docs.NoMissing.From } + config = + { document = Docs.NoMissing.everything + , from = Docs.NoMissing.allModules + } + in + describe "document everything - from everywhere" + [ test "should report an error when a function does not have documentation" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing + +function = 1 +""" + |> Review.Test.run (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing documentation for `function`" + , details = missingElementDetails + , under = "function" + } + ] + , test "should not report an error when a function does have documentation" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing + +{-| documentation -} +function = 1 +""" + |> Review.Test.run (rule config) + |> Review.Test.expectNoErrors + , test "should report an error when a function's documentation is empty" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing + +{-| -} +function = 1 +""" + |> Review.Test.run (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "The documentation for `function` is empty" + , details = [ "Empty documentation is not useful for the users. Please give explanations or examples." ] + , under = "{-| -}" + } + ] + , test "should report an error when a custom type does not have documentation" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing +type CustomType = A +""" + |> Review.Test.run (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing documentation for `CustomType`" + , details = missingElementDetails + , under = "CustomType" + } + ] + , test "should not report an error when a custom type does have documentation" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing +{-| documentation -} +type CustomType = A +""" + |> Review.Test.run (rule config) + |> Review.Test.expectNoErrors + , test "should report an error when a custom type's documentation is empty" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing +{-| -} +type CustomType = A +""" + |> Review.Test.run (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "The documentation for `CustomType` is empty" + , details = [ "Empty documentation is not useful for the users. Please give explanations or examples." ] + , under = "{-| -}" + } + ] + , test "should report an error when a type alias does not have documentation" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing +type alias Alias = A +""" + |> Review.Test.run (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing documentation for `Alias`" + , details = missingElementDetails + , under = "Alias" + } + ] + , test "should not report an error when a type alias does have documentation" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing +{-| documentation -} +type alias Alias = A +""" + |> Review.Test.run (rule config) + |> Review.Test.expectNoErrors + , test "should report an error when a type alias' documentation is empty" <| + \() -> + """module A exposing (..) +{-| module documentation -} +import Thing +{-| -} +type alias Alias = A +""" + |> Review.Test.run (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "The documentation for `Alias` is empty" + , details = [ "Empty documentation is not useful for the users. Please give explanations or examples." ] + , under = "{-| -}" + } + ] + , test "should report an error when a module does not have documentation" <| + \() -> + """module A exposing (..) +import Thing +""" + |> Review.Test.run (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing documentation for module `A`" + , details = missingModuleDetails + , under = "A" + } + ] + , test "should not report an error when a module does have documentation" <| + \() -> + """module A exposing (..) +{-| documentation -} +import Thing +""" + |> Review.Test.run (rule config) + |> Review.Test.expectNoErrors + , test "should report an error when the module's documentation is empty" <| + \() -> + """module A exposing (..) +{-| -} +import Thing +""" + |> Review.Test.run (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "The documentation for module `A` is empty" + , details = [ "Empty documentation is not useful for the users. Please give explanations or examples." ] + , under = "{-| -}" + } + ] + ] + + +everythingFromExposedModulesTests : Test +everythingFromExposedModulesTests = + let + config : { document : Docs.NoMissing.What, from : Docs.NoMissing.From } + config = + { document = Docs.NoMissing.everything + , from = Docs.NoMissing.exposedModules + } + in + describe "document everything - from exposed modules" + [ test "should not report things from non-exposed modules for a package" <| + \() -> + """module NotExposed exposing (..) +import Thing +""" + |> Review.Test.runWithProjectData packageProject (rule config) + |> Review.Test.expectNoErrors + , test "should report things from exposed modules for a package" <| + \() -> + """module Exposed exposing (..) +import Thing +""" + |> Review.Test.runWithProjectData packageProject (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing documentation for module `Exposed`" + , details = missingModuleDetails + , under = "Exposed" + } + ] + ] + + +onlyExposedFromExposedModulesTests : Test +onlyExposedFromExposedModulesTests = + let + config : { document : Docs.NoMissing.What, from : Docs.NoMissing.From } + config = + { document = Docs.NoMissing.onlyExposed + , from = Docs.NoMissing.exposedModules + } + in + describe "document only exposed - from exposed modules" + [ test "should not report non-exposed elements from exposed modules" <| + \() -> + """module Exposed exposing (a) + +{-| module +-} +import Thing + +{-| a +-} +a : () +a = () + +b = () +""" + |> Review.Test.runWithProjectData packageProject (rule config) + |> Review.Test.expectNoErrors + , test "should report exposed elements from exposed modules, using exposing everything" <| + \() -> + """module Exposed exposing (..) +import Thing +function = 1 +type CustomType = Variant +type alias Alias = A +""" + |> Review.Test.runWithProjectData packageProject (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing documentation for module `Exposed`" + , details = missingModuleDetails + , under = "Exposed" + } + , Review.Test.error + { message = "Missing documentation for `function`" + , details = missingElementDetails + , under = "function" + } + , Review.Test.error + { message = "Missing documentation for `CustomType`" + , details = missingElementDetails + , under = "CustomType" + } + , Review.Test.error + { message = "Missing documentation for `Alias`" + , details = missingElementDetails + , under = "Alias" + } + ] + , test "should report exposed elements from exposed modules, using explicit exposing" <| + \() -> + """module Exposed exposing (function, CustomType, Alias) +import Thing +function = 1 +type CustomType = Variant +type alias Alias = A +""" + |> Review.Test.runWithProjectData packageProject (rule config) + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing documentation for module `Exposed`" + , details = missingModuleDetails + , under = "Exposed" + } + , Review.Test.error + { message = "Missing documentation for `function`" + , details = missingElementDetails + , under = "function" + } + |> Review.Test.atExactly { start = { row = 3, column = 1 }, end = { row = 3, column = 9 } } + , Review.Test.error + { message = "Missing documentation for `CustomType`" + , details = missingElementDetails + , under = "CustomType" + } + |> Review.Test.atExactly { start = { row = 4, column = 6 }, end = { row = 4, column = 16 } } + , Review.Test.error + { message = "Missing documentation for `Alias`" + , details = missingElementDetails + , under = "Alias" + } + |> Review.Test.atExactly { start = { row = 5, column = 12 }, end = { row = 5, column = 17 } } + ] + ] + + +packageProject : Project +packageProject = + Project.new + |> Project.addElmJson (createElmJson packageElmJson) + + +packageElmJson : String +packageElmJson = + """ +{ + "type": "package", + "name": "author/package", + "summary": "Summary", + "license": "BSD-3-Clause", + "version": "1.0.0", + "exposed-modules": [ + "Exposed" + ], + "elm-version": "0.19.0 <= v < 0.20.0", + "dependencies": { + "elm/core": "1.0.0 <= v < 2.0.0" + }, + "test-dependencies": {} +}""" + + +createElmJson : String -> { path : String, raw : String, project : Elm.Project.Project } +createElmJson rawElmJson = + case Decode.decodeString Elm.Project.decoder rawElmJson of + Ok elmJson -> + { path = "elm.json" + , raw = rawElmJson + , project = elmJson + } + + Err _ -> + Debug.todo "Invalid elm.json supplied to test" diff --git a/tests/Docs/ReviewAtDocs.elm b/tests/Docs/ReviewAtDocs.elm new file mode 100644 index 00000000..2eb45295 --- /dev/null +++ b/tests/Docs/ReviewAtDocs.elm @@ -0,0 +1,473 @@ +module Docs.ReviewAtDocs exposing (rule) + +{-| + +@docs rule + +-} + +import Dict +import Docs.Utils.ExposedFromProject as ExposedFromProject +import Elm.Project +import Elm.Syntax.Declaration as Declaration exposing (Declaration) +import Elm.Syntax.Exposing as Exposing exposing (Exposing) +import Elm.Syntax.Module as Module exposing (Module) +import Elm.Syntax.Node as Node exposing (Node(..)) +import Elm.Syntax.Range as Range exposing (Range) +import Parser exposing ((|.), (|=), Parser) +import Review.Rule as Rule exposing (Rule) +import Set exposing (Set) + + + +-- TODO Report @docs Thing(..) like in: +-- https://github.com/Holmusk/swagger-decoder/blob/1.0.0/src/Swagger/Types.elm +-- https://package.elm-lang.org/packages/Holmusk/swagger-decoder/latest/Swagger-Types#Scheme +-- TODO Report https://github.com/elm/package.elm-lang.org/issues/311 +-- TODO Report https://github.com/elm/package.elm-lang.org/issues/216 +-- TODO Report @docs in README? + + +{-| Reports problems with the usages of `@docs`. + + config = + [ Docs.ReviewAtDocs.rule + ] + +The aim of this rule is to report problems for documentation in packages that the Elm compiler doesn't report but that +break documentation, and to replicate the same checks for applications so that you can write documentation without +worrying about them getting stale. + +The rule will report issues with malformed `@docs` directives that will cause the documentation to not be displayed properly once published. + + - `@docs` on the first line + +```elm +{-| + +@docs a + +-} +``` + + - Indented `@docs` + +```elm +{-| + + @docs a + +-} +``` + +Once there are no more issues of malformed `@docs`, the rule will report about: + + - Missing `@docs` for exposed elements + + - `@docs` for non-exposed or missing elements + + - Duplicate `@docs` references + + - Usage of `@docs` outside of the module documentation + +If a module does not have _any_ usage of `@docs`, then the rule will not report anything, as the rule will assume the +module is not meant to be documented at this moment in time. An exception is made for exposed modules of a package. + + +## When (not) to enable this rule + +This rule will not be useful if your project is an application and no-one in the team has the habit of writing +package-like documentation. + + +## Try it out + +You can try this rule out by running the following command: + +```bash +elm-review --template jfmengels/elm-review-documentation/example --rules Docs.ReviewAtDocs +``` + +-} +rule : Rule +rule = + Rule.newModuleRuleSchema "Docs.ReviewAtDocs" initialContext + |> Rule.withElmJsonModuleVisitor elmJsonVisitor + |> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor + |> Rule.withCommentsVisitor commentsVisitor + |> Rule.withDeclarationListVisitor (\nodes context -> ( declarationListVisitor nodes context, context )) + |> Rule.fromModuleRuleSchema + + +type alias Context = + { exposedModulesFromProject : Set String + , moduleIsExposed : Bool + , exposedFromModule : Exposing + , hasMalformedDocs : Bool + , docsReferences : List (Node String) + } + + +initialContext : Context +initialContext = + { exposedModulesFromProject = Set.empty + , moduleIsExposed = False + , exposedFromModule = Exposing.All Range.emptyRange + , hasMalformedDocs = False + , docsReferences = [] + } + + + +-- ELM.JSON VISITOR + + +elmJsonVisitor : Maybe Elm.Project.Project -> Context -> Context +elmJsonVisitor maybeProject context = + let + exposedModules : Set String + exposedModules = + case maybeProject of + Just project -> + ExposedFromProject.exposedModules project + + _ -> + Set.empty + in + { context | exposedModulesFromProject = exposedModules } + + + +-- MODULE DEFINITION VISITOR + + +moduleDefinitionVisitor : Node Module -> Context -> ( List nothing, Context ) +moduleDefinitionVisitor node context = + ( [] + , { context + | exposedFromModule = Module.exposingList (Node.value node) + , moduleIsExposed = Set.member (Module.moduleName (Node.value node) |> String.join ".") context.exposedModulesFromProject + } + ) + + + +-- COMMENTS VISITOR + + +commentsVisitor : List (Node String) -> Context -> ( List (Rule.Error {}), Context ) +commentsVisitor nodes context = + case find (Node.value >> String.startsWith "{-|") nodes of + Just (Node range comment) -> + case String.lines comment of + firstLine :: restOfLines -> + let + ( linesThatStartWithAtDocs, linesThatDontStartWithAtDocs ) = + restOfLines + |> List.indexedMap (\index line -> ( index + range.start.row + 1, line )) + |> List.partition (Tuple.second >> String.startsWith "@docs ") + + misformedDocsErrors : List (Rule.Error {}) + misformedDocsErrors = + List.append + (reportDocsOnFirstLine range.start.row firstLine) + (List.concatMap reportIndentedDocs linesThatDontStartWithAtDocs) + in + ( misformedDocsErrors + , { context + | docsReferences = List.concatMap collectDocStatements linesThatStartWithAtDocs + , hasMalformedDocs = not (List.isEmpty misformedDocsErrors) + } + ) + + [] -> + ( [], context ) + + Nothing -> + ( [], context ) + + +reportDocsOnFirstLine : Int -> String -> List (Rule.Error {}) +reportDocsOnFirstLine lineNumber line = + Parser.run (Parser.succeed identity |. Parser.keyword "{-|" |= docsWithSpacesParser lineNumber) line + |> Result.map + (\range -> + [ Rule.error + { message = "Found @docs on the first line" + , details = [ "Using @docs on the first line will make for a broken documentation once published. Please move it to the beginning of the next line." ] + } + range + ] + ) + |> Result.withDefault [] + + +reportIndentedDocs : ( Int, String ) -> List (Rule.Error {}) +reportIndentedDocs ( lineNumber, line ) = + Parser.run (docsWithSpacesParser lineNumber) line + |> Result.map + (\range -> + [ Rule.error + { message = "Found indented @docs" + , details = [ "@docs need to be at the beginning of a line, otherwise they can lead to broken documentation once published. on the first line will make for a broken documentation once published. Please remove the leading spaces" ] + } + range + ] + ) + |> Result.withDefault [] + + +docsWithSpacesParser : Int -> Parser Range +docsWithSpacesParser row = + Parser.succeed + (\startColumn endColumn -> + { start = { row = row, column = startColumn }, end = { row = row, column = endColumn } } + ) + |. Parser.spaces + |= Parser.getCol + |. Parser.keyword "@docs" + |= Parser.getCol + + +collectDocStatements : ( Int, String ) -> List (Node String) +collectDocStatements ( lineNumber, string ) = + Parser.run (docElementsParser lineNumber) string + |> Result.withDefault [] + + +docElementsParser : Int -> Parser (List (Node String)) +docElementsParser startRow = + Parser.succeed identity + |. Parser.keyword "@docs" + |. Parser.spaces + |= Parser.sequence + { start = "" + , separator = "," + , end = "" + , spaces = Parser.spaces + , item = docsItemParser startRow + , trailing = Parser.Forbidden + } + + +docsItemParser : Int -> Parser (Node String) +docsItemParser row = + Parser.succeed + (\startColumn name endColumn -> + Node + { start = { row = row, column = startColumn } + , end = { row = row, column = endColumn } + } + name + ) + |= Parser.getCol + |= Parser.variable + { start = Char.isAlpha + , inner = \c -> Char.isAlphaNum c || c == '_' + , reserved = Set.empty + } + |= Parser.getCol + + + +-- DECLARATION LIST VISITOR + + +declarationListVisitor : List (Node Declaration) -> Context -> List (Rule.Error {}) +declarationListVisitor nodes context = + if context.hasMalformedDocs || (List.isEmpty context.docsReferences && not context.moduleIsExposed) then + List.concatMap errorsForDocsInDeclarationDoc nodes + + else + let + exposedNodes : List (Node String) + exposedNodes = + case context.exposedFromModule of + Exposing.All _ -> + List.filterMap declarationName nodes + + Exposing.Explicit explicit -> + List.map topLevelExposeName explicit + + exposed : Set String + exposed = + Set.fromList (List.map Node.value exposedNodes) + + ( duplicateDocErrors, referencedElements ) = + errorsForDuplicateDocs context.docsReferences + in + List.concat + [ errorsForDocsForNonExposedElements exposed context.docsReferences + , errorsForExposedElementsWithoutADocsReference referencedElements exposedNodes + , List.concatMap errorsForDocsInDeclarationDoc nodes + , duplicateDocErrors + ] + + +errorsForDocsForNonExposedElements : Set String -> List (Node String) -> List (Rule.Error {}) +errorsForDocsForNonExposedElements exposed docsReferences = + docsReferences + |> List.filter (\(Node _ name) -> not (Set.member name exposed)) + |> List.map + (\(Node range name) -> + Rule.error + { message = "Found @docs reference for non-exposed `" ++ name ++ "`" + , details = + [ "I couldn't find this element among the module's exposed elements. Maybe you removed or renamed it recently." + , "Please remove the @docs reference or update the reference to the new name." + ] + } + range + ) + + +errorsForExposedElementsWithoutADocsReference : Set String -> List (Node String) -> List (Rule.Error {}) +errorsForExposedElementsWithoutADocsReference allDocsReferences exposedNodes = + exposedNodes + |> List.filter (\(Node _ name) -> not (Set.member name allDocsReferences)) + |> List.map + (\(Node range name) -> + Rule.error + { message = "Missing @docs reference for exposed `" ++ name ++ "`" + , details = + [ "There is no @docs reference for this element. Maybe you exposed or renamed it recently." + , "Please add a @docs reference to it the module documentation (the one at the top of the module) like this:" + , """{-| +@docs """ ++ name ++ """ +-}""" + ] + } + range + ) + + +errorsForDocsInDeclarationDoc : Node Declaration -> List (Rule.Error {}) +errorsForDocsInDeclarationDoc node = + case docForDeclaration node of + Just ( declarationType, Node docRange docContent ) -> + indexedConcatMap + (\lineNumber lineContent -> + lineContent + |> Parser.run (docsWithSpacesParser (lineNumber + docRange.start.row)) + |> Result.map + (\range -> + [ Rule.error + { message = "Found usage of @docs in a " ++ declarationType ++ " documentation" + , details = [ "@docs can only be used in the module's documentation. You should remove this @docs and move it there." ] + } + range + ] + ) + |> Result.withDefault [] + ) + (String.lines docContent) + + Nothing -> + [] + + +docForDeclaration : Node Declaration -> Maybe ( String, Node String ) +docForDeclaration node = + case Node.value node of + Declaration.FunctionDeclaration function -> + Maybe.map (Tuple.pair "function") function.documentation + + Declaration.AliasDeclaration typeAlias -> + Maybe.map (Tuple.pair "type") typeAlias.documentation + + Declaration.CustomTypeDeclaration customType -> + Maybe.map (Tuple.pair "type") customType.documentation + + Declaration.PortDeclaration _ -> + -- TODO Support port declaration in elm-syntax v8 + Nothing + + Declaration.InfixDeclaration _ -> + Nothing + + Declaration.Destructuring _ _ -> + Nothing + + +errorsForDuplicateDocs : List (Node String) -> ( List (Rule.Error {}), Set String ) +errorsForDuplicateDocs docsReferences = + List.foldl + (\(Node range name) ( errors, previouslyFoundNames ) -> + case Dict.get name previouslyFoundNames of + Just lineNumber -> + ( Rule.error + { message = "Found duplicate @docs reference for `element`" + , details = [ "An element should only be referenced once, but I found a previous reference to it on line " ++ String.fromInt lineNumber ++ ". Please remove one of them." ] + } + range + :: errors + , previouslyFoundNames + ) + + Nothing -> + ( errors, Dict.insert name range.start.row previouslyFoundNames ) + ) + ( [], Dict.empty ) + docsReferences + |> Tuple.mapSecond (Dict.keys >> Set.fromList) + + +declarationName : Node Declaration -> Maybe (Node String) +declarationName node = + case Node.value node of + Declaration.FunctionDeclaration function -> + function.declaration |> Node.value |> .name |> Just + + Declaration.AliasDeclaration typeAlias -> + Just typeAlias.name + + Declaration.CustomTypeDeclaration type_ -> + Just type_.name + + Declaration.PortDeclaration signature -> + Just signature.name + + Declaration.InfixDeclaration { operator } -> + Just operator + + Declaration.Destructuring _ _ -> + Nothing + + +topLevelExposeName : Node Exposing.TopLevelExpose -> Node String +topLevelExposeName (Node range topLevelExpose) = + case topLevelExpose of + Exposing.InfixExpose name -> + Node range name + + Exposing.FunctionExpose name -> + Node range name + + Exposing.TypeOrAliasExpose name -> + Node range name + + Exposing.TypeExpose { name } -> + Node range name + + +find : (a -> Bool) -> List a -> Maybe a +find predicate list = + case list of + [] -> + Nothing + + first :: rest -> + if predicate first then + Just first + + else + find predicate rest + + +indexedConcatMap : (Int -> a -> List b) -> List a -> List b +indexedConcatMap function list = + List.foldl + (\a ( index, acc ) -> ( index + 1, List.append (function index a) acc )) + ( 0, [] ) + list + |> Tuple.second diff --git a/tests/Docs/ReviewAtDocsTest.elm b/tests/Docs/ReviewAtDocsTest.elm new file mode 100644 index 00000000..8de0497c --- /dev/null +++ b/tests/Docs/ReviewAtDocsTest.elm @@ -0,0 +1,382 @@ +module Docs.ReviewAtDocsTest exposing (all) + +import Docs.ReviewAtDocs exposing (rule) +import Elm.Project +import Json.Decode +import Review.Project as Project exposing (Project) +import Review.Test +import Test exposing (Test, describe, test) + + +all : Test +all = + describe "Docs.ReviewAtDocs" + [ test "should not report an error when all @docs are correct" <| + \() -> + """module A exposing (D, T, a, b, c) + +{-| Bla bla + +@docs T, a, b +@docs c, D +-} +import B +a = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report an error when an element has a @docs reference but function is not exposed" <| + \() -> + """module A exposing (a, b) + +{-| Bla bla +@docs a, b, notExposed +-} +import B +a = 1 +b = 2 +notExposed = 3 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found @docs reference for non-exposed `notExposed`" + , details = + [ "I couldn't find this element among the module's exposed elements. Maybe you removed or renamed it recently." + , "Please remove the @docs reference or update the reference to the new name." + ] + , under = "notExposed" + } + |> Review.Test.atExactly { start = { row = 4, column = 13 }, end = { row = 4, column = 23 } } + ] + , test "should report an error when an element has a @docs reference but type is not exposed" <| + \() -> + """module A exposing (a, b) + +{-| Bla bla +@docs a, b, NotExposed +-} +import B +a = 1 +b = 2 +type NotExposed = NotExposed +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found @docs reference for non-exposed `NotExposed`" + , details = + [ "I couldn't find this element among the module's exposed elements. Maybe you removed or renamed it recently." + , "Please remove the @docs reference or update the reference to the new name." + ] + , under = "NotExposed" + } + |> Review.Test.atExactly { start = { row = 4, column = 13 }, end = { row = 4, column = 23 } } + ] + , test "should not report an error when an element has a @docs reference and is exposed with exposing (..)" <| + \() -> + """module A exposing (..) + +{-| Bla bla +@docs a, b, Exposed +-} +import B +a = 1 +b = 2 +type Exposed = Exposed +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report an error when an unknown element has a @docs reference, with exposing (..)" <| + \() -> + """module A exposing (..) + +{-| Bla bla +@docs a, b, Exposed +-} +import B +a = 1 +b = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found @docs reference for non-exposed `Exposed`" + , details = + [ "I couldn't find this element among the module's exposed elements. Maybe you removed or renamed it recently." + , "Please remove the @docs reference or update the reference to the new name." + ] + , under = "Exposed" + } + |> Review.Test.atExactly { start = { row = 4, column = 13 }, end = { row = 4, column = 20 } } + ] + , test "should report an error when encountering @docs on the first line of the module documentation (without space)" <| + \() -> + """module A exposing (a) + +{-|@docs a +-} +import B +a = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found @docs on the first line" + , details = [ "Using @docs on the first line will make for a broken documentation once published. Please move it to the beginning of the next line." ] + , under = "@docs" + } + ] + , test "should report an error when encountering @docs on the first line of the module documentation (with space)" <| + \() -> + """module A exposing (a) + +{-| @docs a +-} +import B +a = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found @docs on the first line" + , details = [ "Using @docs on the first line will make for a broken documentation once published. Please move it to the beginning of the next line." ] + , under = "@docs" + } + ] + , test "should report an error when encountering indented @docs" <| + \() -> + """module A exposing (a) + +{-| + @docs a +-} +import B +a = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found indented @docs" + , details = [ "@docs need to be at the beginning of a line, otherwise they can lead to broken documentation once published. on the first line will make for a broken documentation once published. Please remove the leading spaces" ] + , under = "@docs" + } + ] + , test "should report an error when an element is exposed but has no @docs reference" <| + \() -> + """module A exposing (a, b, exposed) + +{-| Bla bla +@docs a, b +-} +import B +a = 1 +b = 2 +exposed = 3 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing @docs reference for exposed `exposed`" + , details = + [ "There is no @docs reference for this element. Maybe you exposed or renamed it recently." + , "Please add a @docs reference to it the module documentation (the one at the top of the module) like this:" + , """{-| +@docs exposed +-}""" + ] + , under = "exposed" + } + |> Review.Test.atExactly { start = { row = 1, column = 26 }, end = { row = 1, column = 33 } } + ] + , test "should not report errors when there is no @docs at all" <| + \() -> + """module A exposing (a) + +{-| Bla bla +-} +import B +a = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report errors for exposed modules of a package even if there are no @docs at all" <| + \() -> + """module Exposed exposing (element) + +{-| Bla bla +-} +import B +element = 1 +""" + |> Review.Test.runWithProjectData package rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Missing @docs reference for exposed `element`" + , details = + [ "There is no @docs reference for this element. Maybe you exposed or renamed it recently." + , "Please add a @docs reference to it the module documentation (the one at the top of the module) like this:" + , """{-| +@docs element +-}""" + ] + , under = "element" + } + |> Review.Test.atExactly { start = { row = 1, column = 26 }, end = { row = 1, column = 33 } } + ] + , test "should report errors for duplicate docs" <| + \() -> + """module Exposed exposing (something, element) + +{-| +@docs element +@docs something, element +-} +import B +element = 1 +something = 2 +""" + |> Review.Test.runWithProjectData package rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found duplicate @docs reference for `element`" + , details = [ "An element should only be referenced once, but I found a previous reference to it on line 4. Please remove one of them." ] + , under = "element" + } + |> Review.Test.atExactly { start = { row = 5, column = 18 }, end = { row = 5, column = 25 } } + ] + , test "should report errors for usage of @docs in function documentation" <| + \() -> + """module A exposing (something, element) +{-| +@docs something, element +-} +import B +{-| +@docs something +-} +element = 1 +something = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found usage of @docs in a function documentation" + , details = [ "@docs can only be used in the module's documentation. You should remove this @docs and move it there." ] + , under = "@docs" + } + |> Review.Test.atExactly { start = { row = 7, column = 1 }, end = { row = 7, column = 6 } } + ] + , test "should report errors for usage of @docs in function documentation even if there are no @docs in the module documentation" <| + \() -> + """module A exposing (something, element) +import B +{-| +@docs something +-} +element = 1 +something = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found usage of @docs in a function documentation" + , details = [ "@docs can only be used in the module's documentation. You should remove this @docs and move it there." ] + , under = "@docs" + } + ] + , test "should report errors for usage of @docs in type alias documentation" <| + \() -> + """module A exposing (something, Element) +{-| +@docs something, Element +-} +import B +{-| +@docs something +-} +type alias Element = {} +something = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found usage of @docs in a type documentation" + , details = [ "@docs can only be used in the module's documentation. You should remove this @docs and move it there." ] + , under = "@docs" + } + |> Review.Test.atExactly { start = { row = 7, column = 1 }, end = { row = 7, column = 6 } } + ] + , test "should report errors for usage of @docs in custom type documentation" <| + \() -> + """module A exposing (something, Element) +{-| +@docs something, Element +-} +import B +{-| +Bla bla bla + +@docs something +-} +type Element = Element +something = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Found usage of @docs in a type documentation" + , details = [ "@docs can only be used in the module's documentation. You should remove this @docs and move it there." ] + , under = "@docs" + } + |> Review.Test.atExactly { start = { row = 9, column = 1 }, end = { row = 9, column = 6 } } + ] + , test "should not report mention of @docs after words" <| + \() -> + """module A exposing (a) +{-| +@docs a +-} +import B + +{-| Reports problems with the usages of `@docs`. +-} +a = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + ] + + +package : Project +package = + case Json.Decode.decodeString Elm.Project.decoder elmJson of + Ok project -> + Project.new + |> Project.addElmJson + { path = "elm.json" + , raw = elmJson + , project = project + } + + Err err -> + Debug.todo ("Invalid elm.json supplied to test: " ++ Debug.toString err) + + +elmJson : String +elmJson = + """{ + "type": "package", + "name": "author/package", + "summary": "Summary", + "license": "BSD-3-Clause", + "version": "1.0.0", + "exposed-modules": [ + "Exposed" + ], + "elm-version": "0.19.0 <= v < 0.20.0", + "dependencies": { + "elm/core": "1.0.0 <= v < 2.0.0" + }, + "test-dependencies": {} +}""" diff --git a/tests/Docs/ReviewLinksAndSections.elm b/tests/Docs/ReviewLinksAndSections.elm new file mode 100644 index 00000000..25ed9530 --- /dev/null +++ b/tests/Docs/ReviewLinksAndSections.elm @@ -0,0 +1,821 @@ +module Docs.ReviewLinksAndSections exposing (rule) + +{-| + +@docs rule + +-} + +import Dict exposing (Dict) +import Docs.Utils.Link as Link +import Docs.Utils.Slug as Slug +import Elm.Module +import Elm.Package +import Elm.Project +import Elm.Syntax.Declaration as Declaration exposing (Declaration) +import Elm.Syntax.Documentation exposing (Documentation) +import Elm.Syntax.Exposing as Exposing +import Elm.Syntax.Module as Module exposing (Module) +import Elm.Syntax.ModuleName exposing (ModuleName) +import Elm.Syntax.Node as Node exposing (Node(..)) +import Elm.Syntax.Range exposing (Range) +import Elm.Version +import Regex exposing (Regex) +import Review.Rule as Rule exposing (Rule) +import Set exposing (Set) + + +{-| Reports problems with links and sections in Elm projects. + + config = + [ Docs.ReviewLinksAndSections.rule + ] + + +## Fail + +Links to missing modules or sections are reported. + + {-| Link to [missing module](Unknown-Module). + -} + a = + 1 + + {-| Link to [missing section](#unknown). + -} + a = + 1 + +In packages, links that would appear in the public documentation and that link to sections not part of the public documentation are reported. + + module Exposed exposing (a) + + import Internal + + {-| Link to [internal details](Internal#section). + -} + a = + 1 + +Sections that would have the same generated id are reported, +so that links don't inadvertently point to the wrong location. + + module A exposing (element, section) + + {-| + + + # Section + + The above conflicts with the id generated + for the `section` value. + + -} + + element = + 1 + + section = + 1 + + +## Success + + module Exposed exposing (a, b) + + import Internal + + {-| Link to [exposed b](#b). + -} + a = + 1 + + b = + 2 + + +## When (not) to enable this rule + +For packages, this rule will be useful to prevent having dead links in the package documentation. + +For applications, this rule will be useful if you have the habit of writing documentation the way you do in Elm packages, +and want to prevent it from going out of date. + +This rule will not be useful if your project is an application and no-one in the team has the habit of writing +package-like documentation. + + +## Try it out + +You can try this rule out by running the following command: + +```bash +elm-review --template jfmengels/elm-review-documentation/example --rules Docs.ReviewLinksAndSections +``` + + +## Thanks + +Thanks to @lue-bird for helping out with this rule. + +-} +rule : Rule +rule = + Rule.newProjectRuleSchema "Docs.ReviewLinksAndSections" initialProjectContext + |> Rule.withElmJsonProjectVisitor elmJsonVisitor + |> Rule.withReadmeProjectVisitor readmeVisitor + |> Rule.withModuleVisitor moduleVisitor + |> Rule.withModuleContextUsingContextCreator + { fromProjectToModule = fromProjectToModule + , fromModuleToProject = fromModuleToProject + , foldProjectContexts = foldProjectContexts + } + |> Rule.withFinalProjectEvaluation finalEvaluation + |> Rule.fromProjectRuleSchema + + +type alias ProjectContext = + { fileLinksAndSections : List FileLinksAndSections + , packageNameAndVersion : Maybe { name : String, version : String } + , exposedModules : Set ModuleName + } + + +initialProjectContext : ProjectContext +initialProjectContext = + { fileLinksAndSections = [] + , packageNameAndVersion = Nothing + , exposedModules = Set.empty + } + + +type alias FileLinksAndSections = + { moduleName : ModuleName + , fileKey : FileKey + , sections : List Section + , links : List MaybeExposedLink + } + + +type FileKey + = ModuleKey Rule.ModuleKey + | ReadmeKey Rule.ReadmeKey + + +type alias ModuleContext = + { isModuleExposed : Bool + , exposedElements : Set String + , moduleName : ModuleName + , commentSections : List SectionWithRange + , sections : List Section + , links : List MaybeExposedLink + } + + +type alias Section = + { slug : String + , isExposed : Bool + } + + +type MaybeExposedLink + = MaybeExposedLink MaybeExposedLinkData + + +type alias MaybeExposedLinkData = + { link : Link.Link + , linkRange : Range + , isExposed : Bool + } + + +fromProjectToModule : Rule.ContextCreator ProjectContext ModuleContext +fromProjectToModule = + Rule.initContextCreator + (\metadata projectContext -> + let + moduleName : ModuleName + moduleName = + Rule.moduleNameFromMetadata metadata + in + { isModuleExposed = Set.member moduleName projectContext.exposedModules + , exposedElements = Set.empty + , moduleName = moduleName + , commentSections = [] + , sections = [] + , links = [] + } + ) + |> Rule.withMetadata + + +fromModuleToProject : Rule.ContextCreator ModuleContext ProjectContext +fromModuleToProject = + Rule.initContextCreator + (\moduleKey moduleContext -> + { fileLinksAndSections = + [ { moduleName = moduleContext.moduleName + , fileKey = ModuleKey moduleKey + , sections = moduleContext.sections + , links = moduleContext.links + } + ] + , packageNameAndVersion = Nothing + , exposedModules = Set.empty + } + ) + |> Rule.withModuleKey + + +foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext +foldProjectContexts newContext previousContext = + { fileLinksAndSections = List.append newContext.fileLinksAndSections previousContext.fileLinksAndSections + , packageNameAndVersion = previousContext.packageNameAndVersion + , exposedModules = previousContext.exposedModules + } + + +moduleVisitor : Rule.ModuleRuleSchema schemaState ModuleContext -> Rule.ModuleRuleSchema { schemaState | hasAtLeastOneVisitor : () } ModuleContext +moduleVisitor schema = + schema + |> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor + |> Rule.withCommentsVisitor commentsVisitor + |> Rule.withDeclarationListVisitor declarationListVisitor + + + +-- ELM.JSON VISITOR + + +elmJsonVisitor : Maybe { a | project : Elm.Project.Project } -> ProjectContext -> ( List nothing, ProjectContext ) +elmJsonVisitor maybeElmJson projectContext = + case Maybe.map .project maybeElmJson of + Just (Elm.Project.Package { name, version, exposed }) -> + ( [] + , { projectContext + | packageNameAndVersion = Just { name = Elm.Package.toString name, version = Elm.Version.toString version } + , exposedModules = listExposedModules exposed + } + ) + + _ -> + ( [], projectContext ) + + +listExposedModules : Elm.Project.Exposed -> Set ModuleName +listExposedModules exposed = + let + exposedModules : List ModuleName + exposedModules = + exposedModulesFromPackageAsList exposed + |> List.map (Elm.Module.toString >> String.split ".") + in + Set.fromList ([] :: exposedModules) + + +exposedModulesFromPackageAsList : Elm.Project.Exposed -> List Elm.Module.Name +exposedModulesFromPackageAsList exposed = + case exposed of + Elm.Project.ExposedList list -> + list + + Elm.Project.ExposedDict list -> + List.concatMap Tuple.second list + + + +-- README VISITOR + + +readmeVisitor : Maybe { readmeKey : Rule.ReadmeKey, content : String } -> ProjectContext -> ( List (Rule.Error { useErrorForModule : () }), ProjectContext ) +readmeVisitor maybeReadmeInfo projectContext = + case maybeReadmeInfo of + Just { readmeKey, content } -> + let + isReadmeExposed : Bool + isReadmeExposed = + Set.member [] projectContext.exposedModules + + sectionsAndLinks : { titleSections : List SectionWithRange, links : List MaybeExposedLink } + sectionsAndLinks = + findSectionsAndLinks + [] + isReadmeExposed + { content = content + , startRow = 1 + } + in + ( duplicateSectionErrors Set.empty sectionsAndLinks.titleSections + |> List.map (Rule.errorForReadme readmeKey duplicateSectionErrorDetails) + , { fileLinksAndSections = + { moduleName = [] + , fileKey = ReadmeKey readmeKey + , sections = List.map removeRangeFromSection sectionsAndLinks.titleSections + , links = sectionsAndLinks.links + } + :: projectContext.fileLinksAndSections + , packageNameAndVersion = projectContext.packageNameAndVersion + , exposedModules = projectContext.exposedModules + } + ) + + Nothing -> + ( [], projectContext ) + + + +-- MODULE DEFINITION VISITOR + + +moduleDefinitionVisitor : Node Module -> ModuleContext -> ( List nothing, ModuleContext ) +moduleDefinitionVisitor node context = + case Module.exposingList (Node.value node) of + Exposing.All _ -> + -- We'll keep `exposedElements` empty, which will make `declarationListVisitor` fill it with the known + -- declarations. + ( [], context ) + + Exposing.Explicit exposed -> + ( [], { context | exposedElements = Set.fromList (List.map exposedName exposed) } ) + + +exposedName : Node Exposing.TopLevelExpose -> String +exposedName node = + case Node.value node of + Exposing.InfixExpose string -> + string + + Exposing.FunctionExpose string -> + string + + Exposing.TypeOrAliasExpose string -> + string + + Exposing.TypeExpose exposedType -> + exposedType.name + + + +-- COMMENTS VISITOR + + +commentsVisitor : List (Node String) -> ModuleContext -> ( List nothing, ModuleContext ) +commentsVisitor comments context = + let + docs : List (Node String) + docs = + List.filter (Node.value >> String.startsWith "{-|") comments + + sectionsAndLinks : List { titleSections : List SectionWithRange, links : List MaybeExposedLink } + sectionsAndLinks = + List.map + (\doc -> + findSectionsAndLinks + context.moduleName + context.isModuleExposed + { content = Node.value doc, startRow = (Node.range doc).start.row } + ) + docs + in + ( [] + , { isModuleExposed = context.isModuleExposed + , exposedElements = context.exposedElements + , moduleName = context.moduleName + , commentSections = List.concatMap .titleSections sectionsAndLinks + , sections = + List.append + (List.concatMap (.titleSections >> List.map removeRangeFromSection) sectionsAndLinks) + context.sections + , links = List.append (List.concatMap .links sectionsAndLinks) context.links + } + ) + + + +-- DECLARATION VISITOR + + +declarationListVisitor : List (Node Declaration) -> ModuleContext -> ( List (Rule.Error {}), ModuleContext ) +declarationListVisitor declarations context = + let + exposedElements : Set String + exposedElements = + if Set.isEmpty context.exposedElements then + Set.fromList (List.filterMap nameOfDeclaration declarations) + + else + context.exposedElements + + knownSections : List { slug : String, isExposed : Bool } + knownSections = + List.append + (List.map (\slug -> { slug = slug, isExposed = True }) (Set.toList exposedElements)) + context.sections + + sectionsAndLinks : List { titleSections : List SectionWithRange, links : List MaybeExposedLink } + sectionsAndLinks = + List.map + (findSectionsAndLinksForDeclaration + context.moduleName + (if context.isModuleExposed then + exposedElements + + else + Set.empty + ) + ) + declarations + + titleSections : List SectionWithRange + titleSections = + List.concatMap .titleSections sectionsAndLinks + in + ( duplicateSectionErrors exposedElements (List.append titleSections context.commentSections) + |> List.map (Rule.error duplicateSectionErrorDetails) + , { isModuleExposed = context.isModuleExposed + , exposedElements = exposedElements + , moduleName = context.moduleName + , commentSections = context.commentSections + , sections = List.append (List.map removeRangeFromSection titleSections) knownSections + , links = List.append (List.concatMap .links sectionsAndLinks) context.links + } + ) + + +duplicateSectionErrors : Set String -> List SectionWithRange -> List Range +duplicateSectionErrors exposedElements sections = + List.foldl + (\{ slug, range } { errors, knownSections } -> + if Set.member slug knownSections then + { errors = range :: errors + , knownSections = knownSections + } + + else + { errors = errors + , knownSections = Set.insert slug knownSections + } + ) + { errors = [], knownSections = exposedElements } + sections + |> .errors + + +extractSlugsFromHeadings : { content : String, startRow : Int } -> List (Node String) +extractSlugsFromHeadings doc = + doc.content + |> String.lines + |> List.indexedMap + (\lineNumber line -> + Regex.find specialsToHash line + |> List.concatMap .submatches + |> List.filterMap identity + |> List.map + (\slug -> + Node + { start = { row = lineNumber + doc.startRow, column = 1 } + , end = { row = lineNumber + doc.startRow, column = String.length line + 1 } + } + (Slug.toSlug slug) + ) + ) + |> List.concat + + +specialsToHash : Regex +specialsToHash = + "^#{1,6}\\s+(.*)$" + |> Regex.fromString + |> Maybe.withDefault Regex.never + + +nameOfDeclaration : Node Declaration -> Maybe String +nameOfDeclaration node = + case Node.value node of + Declaration.FunctionDeclaration { declaration } -> + declaration + |> Node.value + |> .name + |> Node.value + |> Just + + Declaration.AliasDeclaration { name } -> + Just (Node.value name) + + Declaration.CustomTypeDeclaration { name } -> + Just (Node.value name) + + Declaration.PortDeclaration { name } -> + Just (Node.value name) + + Declaration.InfixDeclaration { operator } -> + Just (Node.value operator) + + Declaration.Destructuring _ _ -> + Nothing + + +docOfDeclaration : Declaration -> Maybe (Node Documentation) +docOfDeclaration declaration = + case declaration of + Declaration.FunctionDeclaration { documentation } -> + documentation + + Declaration.AliasDeclaration { documentation } -> + documentation + + Declaration.CustomTypeDeclaration { documentation } -> + documentation + + Declaration.PortDeclaration _ -> + Nothing + + Declaration.InfixDeclaration _ -> + Nothing + + Declaration.Destructuring _ _ -> + Nothing + + +findSectionsAndLinksForDeclaration : ModuleName -> Set String -> Node Declaration -> { titleSections : List SectionWithRange, links : List MaybeExposedLink } +findSectionsAndLinksForDeclaration currentModuleName exposedElements declaration = + case docOfDeclaration (Node.value declaration) of + Just doc -> + let + name : String + name = + nameOfDeclaration declaration + |> Maybe.withDefault "" + + isExposed : Bool + isExposed = + Set.member name exposedElements + in + findSectionsAndLinks + currentModuleName + isExposed + { content = Node.value doc, startRow = (Node.range doc).start.row } + + Nothing -> + { titleSections = [], links = [] } + + +type alias SectionWithRange = + { slug : String + , range : Range + , isExposed : Bool + } + + +removeRangeFromSection : SectionWithRange -> Section +removeRangeFromSection { slug, isExposed } = + { slug = slug + , isExposed = isExposed + } + + +findSectionsAndLinks : ModuleName -> Bool -> { content : String, startRow : Int } -> { titleSections : List SectionWithRange, links : List MaybeExposedLink } +findSectionsAndLinks currentModuleName isExposed doc = + let + titleSections : List SectionWithRange + titleSections = + extractSlugsFromHeadings doc + |> List.map + (\slug -> + { slug = Node.value slug + , range = Node.range slug + , isExposed = isExposed + } + ) + + links : List MaybeExposedLink + links = + Link.findLinks (doc.startRow - 1) currentModuleName doc.content + |> List.map + (\link -> + MaybeExposedLink + { link = Node.value link + , linkRange = Node.range link + , isExposed = isExposed + } + ) + in + { titleSections = titleSections + , links = links + } + + + +-- FINAL EVALUATION + + +finalEvaluation : ProjectContext -> List (Rule.Error { useErrorForModule : () }) +finalEvaluation projectContext = + let + sectionsPerModule : Dict ModuleName (List Section) + sectionsPerModule = + projectContext.fileLinksAndSections + |> List.map (\module_ -> ( module_.moduleName, module_.sections )) + |> Dict.fromList + in + List.concatMap (errorsForFile projectContext sectionsPerModule) projectContext.fileLinksAndSections + + +errorsForFile : ProjectContext -> Dict ModuleName (List Section) -> FileLinksAndSections -> List (Rule.Error scope) +errorsForFile projectContext sectionsPerModule fileLinksAndSections = + List.filterMap + (errorForFile projectContext sectionsPerModule fileLinksAndSections) + fileLinksAndSections.links + + +errorForFile : ProjectContext -> Dict ModuleName (List Section) -> FileLinksAndSections -> MaybeExposedLink -> Maybe (Rule.Error scope) +errorForFile projectContext sectionsPerModule fileLinksAndSections (MaybeExposedLink maybeExposedLink) = + case maybeExposedLink.link.file of + Link.ModuleTarget moduleName -> + reportErrorForModule projectContext sectionsPerModule fileLinksAndSections maybeExposedLink moduleName + + Link.ReadmeTarget -> + reportErrorForReadme sectionsPerModule fileLinksAndSections.fileKey maybeExposedLink + + Link.PackagesTarget packageTarget -> + reportErrorsForPackagesTarget projectContext sectionsPerModule fileLinksAndSections maybeExposedLink packageTarget + + Link.External target -> + reportErrorsForExternalTarget (projectContext.packageNameAndVersion == Nothing) fileLinksAndSections.fileKey maybeExposedLink.linkRange target + + +reportErrorsForPackagesTarget : ProjectContext -> Dict ModuleName (List Section) -> FileLinksAndSections -> MaybeExposedLinkData -> { name : String, version : String, subTarget : Link.SubTarget } -> Maybe (Rule.Error scope) +reportErrorsForPackagesTarget projectContext sectionsPerModule fileLinksAndSections maybeExposedLink { name, version, subTarget } = + case projectContext.packageNameAndVersion of + Just currentPackage -> + if name == currentPackage.name && (version == "latest" || version == currentPackage.version) then + reportErrorForCurrentPackageSubTarget projectContext sectionsPerModule fileLinksAndSections maybeExposedLink subTarget + + else + Nothing + + Nothing -> + Nothing + + +reportErrorForCurrentPackageSubTarget : ProjectContext -> Dict ModuleName (List Section) -> FileLinksAndSections -> MaybeExposedLinkData -> Link.SubTarget -> Maybe (Rule.Error scope) +reportErrorForCurrentPackageSubTarget projectContext sectionsPerModule fileLinksAndSections maybeExposedLink subTarget = + case subTarget of + Link.ModuleSubTarget moduleName -> + reportErrorForModule projectContext sectionsPerModule fileLinksAndSections maybeExposedLink moduleName + + Link.ReadmeSubTarget -> + reportErrorForReadme sectionsPerModule fileLinksAndSections.fileKey maybeExposedLink + + +reportErrorForModule : ProjectContext -> Dict ModuleName (List Section) -> FileLinksAndSections -> MaybeExposedLinkData -> ModuleName -> Maybe (Rule.Error scope) +reportErrorForModule projectContext sectionsPerModule fileLinksAndSections maybeExposedLink moduleName = + case Dict.get moduleName sectionsPerModule of + Just existingSections -> + if Set.member fileLinksAndSections.moduleName projectContext.exposedModules && not (Set.member moduleName projectContext.exposedModules) then + Just (reportLinkToNonExposedModule fileLinksAndSections.fileKey maybeExposedLink.linkRange) + + else + reportIfMissingSection fileLinksAndSections.fileKey existingSections maybeExposedLink + + Nothing -> + Just (reportUnknownModule fileLinksAndSections.fileKey moduleName maybeExposedLink.linkRange) + + +reportErrorForReadme : Dict (List comparable) (List Section) -> FileKey -> MaybeExposedLinkData -> Maybe (Rule.Error scope) +reportErrorForReadme sectionsPerModule fileKey maybeExposedLink = + case Dict.get [] sectionsPerModule of + Just existingSections -> + reportIfMissingSection fileKey existingSections maybeExposedLink + + Nothing -> + Just (reportLinkToMissingReadme fileKey maybeExposedLink.linkRange) + + +reportErrorsForExternalTarget : Bool -> FileKey -> Range -> String -> Maybe (Rule.Error scope) +reportErrorsForExternalTarget isApplication fileKey linkRange target = + if isApplication || String.contains "://" target then + Nothing + + else + Just (reportLinkToExternalResourceWithoutProtocol fileKey linkRange) + + +reportIfMissingSection : FileKey -> List Section -> MaybeExposedLinkData -> Maybe (Rule.Error scope) +reportIfMissingSection fileKey existingSectionsForTargetFile { isExposed, linkRange, link } = + case link.slug of + Just "" -> + Just (reportLinkWithEmptySlug fileKey linkRange) + + Just slug -> + case find (\section -> section.slug == slug) existingSectionsForTargetFile of + Just section -> + if isExposed && not section.isExposed then + Just (reportLinkToNonExposedSection fileKey linkRange) + + else + Nothing + + Nothing -> + Just (reportLink fileKey linkRange) + + Nothing -> + Nothing + + +reportLink : FileKey -> Range -> Rule.Error scope +reportLink fileKey range = + reportForFile fileKey + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + } + range + + +reportLinkToNonExposedModule : FileKey -> Range -> Rule.Error scope +reportLinkToNonExposedModule fileKey range = + reportForFile fileKey + { message = "Link in public documentation points to non-exposed module" + , details = [ "Users will not be able to follow the link." ] + } + range + + +reportLinkToNonExposedSection : FileKey -> Range -> Rule.Error scope +reportLinkToNonExposedSection fileKey range = + reportForFile fileKey + { message = "Link in public documentation points to non-exposed section" + , details = [ "Users will not be able to follow the link." ] + } + range + + +reportLinkWithEmptySlug : FileKey -> Range -> Rule.Error scope +reportLinkWithEmptySlug fileKey range = + reportForFile fileKey + { message = "Link to empty section is unnecessary" + , details = [ "Links to # not followed by an id don't provide any value to the user. I suggest to either strip the # or remove the link." ] + } + range + + +reportUnknownModule : FileKey -> ModuleName -> Range -> Rule.Error scope +reportUnknownModule fileKey moduleName range = + reportForFile fileKey + { message = "Link points to non-existing module " ++ String.join "." moduleName + , details = [ "This is a dead link." ] + } + range + + +reportLinkToMissingReadme : FileKey -> Range -> Rule.Error scope +reportLinkToMissingReadme fileKey range = + reportForFile fileKey + { message = "Link points to missing README" + , details = [ "elm-review only looks for a 'README.md' located next to your 'elm.json'. Maybe it's positioned elsewhere or named differently?" ] + } + range + + +reportLinkToExternalResourceWithoutProtocol : FileKey -> Range -> Rule.Error scope +reportLinkToExternalResourceWithoutProtocol fileKey range = + reportForFile fileKey + { message = "Link to unknown resource without a protocol" + , details = + [ "I have trouble figuring out what kind of resource is linked here." + , "If it should link to a module, then they should be in the form 'Some-Module-Name'." + , "If it's a link to an external resource, they should start with a protocol, like `https://www.fruits.com`, otherwise the link will point to an unknown resource on package.elm-lang.org." + ] + } + range + + +duplicateSectionErrorDetails : { message : String, details : List String } +duplicateSectionErrorDetails = + { message = "Duplicate section" + , details = [ "There are multiple sections that will result in the same id, meaning that links may point towards the wrong element." ] + } + + +reportForFile : FileKey -> { message : String, details : List String } -> Range -> Rule.Error scope +reportForFile fileKey = + case fileKey of + ModuleKey moduleKey -> + Rule.errorForModule moduleKey + + ReadmeKey readmeKey -> + Rule.errorForReadme readmeKey + + +find : (a -> Bool) -> List a -> Maybe a +find predicate list = + case list of + [] -> + Nothing + + first :: rest -> + if predicate first then + Just first + + else + find predicate rest diff --git a/tests/Docs/ReviewLinksAndSectionsTest.elm b/tests/Docs/ReviewLinksAndSectionsTest.elm new file mode 100644 index 00000000..2f285552 --- /dev/null +++ b/tests/Docs/ReviewLinksAndSectionsTest.elm @@ -0,0 +1,1133 @@ +module Docs.ReviewLinksAndSectionsTest exposing (all) + +import Docs.ReviewLinksAndSections exposing (rule) +import Elm.Project +import Json.Decode +import Review.Project as Project exposing (Project) +import Review.Test +import Test exposing (Test, describe, test) + + + +-- TODO Report links to dependencies? +-- TODO Force links to dependencies to be for the minimal version? +-- TODO Report unmatched `[foo]`? +-- TODO Report unused `[foo]: #b` links? (only if there are no "unmatched errors") + + +all : Test +all = + describe "Docs.ReviewLinksAndSections" + [ localLinkTests + , linksToOtherFilesTest + , linksDependingOnExposition + , linksThroughPackageRegistryTest + , duplicateSectionsTests + , unnecessaryLinksTests + , externalResourcesTests + , linksWithAltTextTests + ] + + +localLinkTests : Test +localLinkTests = + describe "Local links" + [ test "should not report link to an existing sibling section from declaration documentation" <| + \() -> + """module A exposing (a, b) +b = 1 + +{-| [link](#b) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report a link to an unknown sibling section from declaration documentation" <| + \() -> + """module A exposing (a) +{-| [link](#b) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#b" + } + ] + , test "should not report link to an existing sibling section from module documentation" <| + \() -> + """module A exposing (a, b) +{-| [link](#b) +-} + +import B +b = 1 +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report a link to an unknown sibling section from module documentation" <| + \() -> + """module A exposing (a) +{-| [link](#b) +-} + +import B +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#b" + } + ] + , test "should not report link to an existing sibling section from port documentation" <| + \() -> + """module A exposing (a, b) +b = 1 +{-| [link](#b) +-} +port a : String -> Cmd msg +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report a link to an unknown sibling section from port documentation" <| + \() -> + """module A exposing (a) +{-| [link](#b) +-} +port a : String -> Cmd msg +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#b" + } + ] + , test "should not report a link to a known sibling section from declaration documentation when everything is exposed" <| + \() -> + """module A exposing (..) +b = 1 + +{-| [link](#b) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report a link to an unknown sibling section from declaration documentation when everything is exposed" <| + \() -> + """module A exposing (..) +{-| [link](#b) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#b" + } + ] + , test "should not report link references that target existing sections" <| + \() -> + """module A exposing (a, b) +b = 1 +{-| this is a [link] reference + +[link]: #b +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report link references that link to missing sections" <| + \() -> + """module A exposing (..) +{-| this is a [link] reference + +[link]: #b + +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#b" + } + ] + , test "should consider an exposed type alias to be an existing section" <| + \() -> + """module A exposing (..) +type alias B = {} +{-| [link](#B) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should consider an exposed custom type to be an existing section" <| + \() -> + """module A exposing (..) +type B = C +{-| [link](#B) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should consider an exposed port to be an existing section" <| + \() -> + """module A exposing (..) +port b : Int -> Cmd msg + +{-| [link](#b) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should consider an infix declaration to be an existing section (operator exists)" <| + \() -> + """module A exposing (..) +infix right 5 (++) = append + +{-| [link](#++) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should consider an infix declaration to be an existing section (operator doesn't exist)" <| + \() -> + """module A exposing (..) +{-| [link](#++) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#++" + } + ] + , test "should consider a header inside a function documentation comment to be an existing section (h1)" <| + \() -> + """module A exposing (..) +{-| +# Section +-} +b = 1 +{-| [link](#section) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should consider a header inside a function documentation comment to be an existing section (h2)" <| + \() -> + """module A exposing (..) +{-| +## Section +-} +b = 1 +{-| [link](#section) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should not consider a 'h7' header inside a function documentation comment to be an existing section" <| + \() -> + """module A exposing (..) +{-| +####### Section +-} +b = 1 +{-| [link](#section) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#section" + } + ] + , test "should consider a header inside a module documentation comment to be an existing section" <| + \() -> + """module A exposing (..) +{-| +# Section +-} + +import B + +b = 1 +{-| [link](#section) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should slugify complex headings" <| + \() -> + """module A exposing (..) +{-| +# Section *with* ~some~ _spaces_ and\\_ $thi.ngs . links + +### `section` +### question? + +-} +b = 1 +{-| +[1](#section-_with_-some-_spaces_-and-_-thi-ngs-links) +[2](#-section-) +[3](#question-) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report links to unknown modules" <| + \() -> + """module A exposing (..) +{-| [link](B-C) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to non-existing module B.C" + , details = [ "This is a dead link." ] + , under = "B-C" + } + ] + , test "should report multiple links on the same line" <| + \() -> + """module A exposing (a) +{-| [link](#b) [link](#c) [link](#d) [link](#e) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#b" + } + , Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#c" + } + , Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#d" + } + , Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#e" + } + ] + ] + + +linksToOtherFilesTest : Test +linksToOtherFilesTest = + describe "Links to other files" + [ test "should report links to sections in unknown modules" <| + \() -> + """module A exposing (..) +{-| [link](B#b) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to non-existing module B" + , details = [ "This is a dead link." ] + , under = "B#b" + } + ] + , test "should not report links to existing sections in a different module" <| + \() -> + [ """module A exposing (..) +{-| [link](B#b) +-} +a = 2 +""", """module B exposing (b) +b = 1 +""" ] + |> Review.Test.runOnModules rule + |> Review.Test.expectNoErrors + , test "should report links to missing sections in a different module" <| + \() -> + [ """module A exposing (..) +{-| [link](B#b) +-} +a = 2 +""", """module B exposing (c) +c = 1 +""" ] + |> Review.Test.runOnModules rule + |> Review.Test.expectErrorsForModules + [ ( "A" + , [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "B#b" + } + ] + ) + ] + , test "should not report links to existing sections in a different module (using ./)" <| + \() -> + [ """module A exposing (..) +{-| [link](./B#b) +-} +a = 2 +""", """module B exposing (b) +b = 1 +""" ] + |> Review.Test.runOnModules rule + |> Review.Test.expectNoErrors + , test "should report links to missing sections in a different module (using ./)" <| + \() -> + [ """module A exposing (..) +{-| [link](./B#b) +-} +a = 2 +""", """module B exposing (c) +c = 1 +""" ] + |> Review.Test.runOnModules rule + |> Review.Test.expectErrorsForModules + [ ( "A" + , [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "./B#b" + } + ] + ) + ] + , test "should not report links to existing sections inside the README" <| + \() -> + """module A exposing (a) +a = 2 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "# A\n[link](#a)" } Project.new) + rule + |> Review.Test.expectNoErrors + , test "should report links to missing sections inside the README" <| + \() -> + """module A exposing (a) +a = 2 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "# A\n[link](#b)" } Project.new) + rule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "#b" + } + ] + , test "should not report links to existing sections inside the README (using ./)" <| + \() -> + """module A exposing (a) +a = 2 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "# A\n[link](./#a)" } Project.new) + rule + |> Review.Test.expectNoErrors + , test "should report links to missing sections inside the README (using ./)" <| + \() -> + """module A exposing (a) +a = 2 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "# A\n[link](./#b)" } Project.new) + rule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "./#b" + } + ] + , test "should not report links to existing sections from another module inside the README" <| + \() -> + """module A exposing (a) +a = 2 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "[link](./A#a)" } Project.new) + rule + |> Review.Test.expectNoErrors + , test "should report links to missing sections from another module inside the README" <| + \() -> + """module A exposing (a) +a = 2 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "[link](./A#b)" } Project.new) + rule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "./A#b" + } + ] + , test "should report links to README when there is no README (without slug)" <| + \() -> + """module A exposing (a) +{-| [link](./) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to missing README" + , details = [ "elm-review only looks for a 'README.md' located next to your 'elm.json'. Maybe it's positioned elsewhere or named differently?" ] + , under = "./" + } + ] + , test "should report links to README when there is no README (with slug)" <| + \() -> + """module A exposing (a) +{-| [link](./#b) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to missing README" + , details = [ "elm-review only looks for a 'README.md' located next to your 'elm.json'. Maybe it's positioned elsewhere or named differently?" ] + , under = "./#b" + } + ] + ] + + +linksThroughPackageRegistryTest : Test +linksThroughPackageRegistryTest = + describe "Links through the package registry" + [ test "should report links to unknown modules for the current version" <| + \() -> + [ """module A exposing (..) +{-| [link](https://package.elm-lang.org/packages/author/package/1.0.0/Unknown) +-} +a = 2 +""" ] + |> Review.Test.runOnModulesWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to non-existing module Unknown" + , details = [ "This is a dead link." ] + , under = "https://package.elm-lang.org/packages/author/package/1.0.0/Unknown" + } + ] + , test "should not report links to known modules for the current version" <| + \() -> + [ """module A exposing (..) +{-| [link](https://package.elm-lang.org/packages/author/package/1.0.0/A) +-} +a = 2 +""" ] + |> Review.Test.runOnModulesWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectNoErrors + , test "should not report links to known modules for the current version (with trailing slash)" <| + \() -> + [ """module A exposing (..) +{-| [link](https://package.elm-lang.org/packages/author/package/1.0.0/A/) +-} +a = 2 +""" ] + |> Review.Test.runOnModulesWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectNoErrors + , test "should report links to unknown modules for latest" <| + \() -> + """module A exposing (..) +{-| [link](https://package.elm-lang.org/packages/author/package/latest/Unknown) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to non-existing module Unknown" + , details = [ "This is a dead link." ] + , under = "https://package.elm-lang.org/packages/author/package/latest/Unknown" + } + ] + , test "should not report links for different version of the package" <| + \() -> + """module A exposing (..) +{-| [link](https://package.elm-lang.org/packages/author/package/2.3.4/Unknown) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectNoErrors + , test "should report links to unknown section of README" <| + \() -> + """module A exposing (..) +{-| [link](https://package.elm-lang.org/packages/author/package/1.0.0/#unknown) +-} +a = 2 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "# A" } packageProjectWithoutFiles) + rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to a non-existing section or element" + , details = [ "This is a dead link." ] + , under = "https://package.elm-lang.org/packages/author/package/1.0.0/#unknown" + } + ] + , test "should not report links to known sections of README" <| + \() -> + """module A exposing (..) +{-| [link](https://package.elm-lang.org/packages/author/package/2.3.4/#a) +-} +a = 2 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "# A" } packageProjectWithoutFiles) + rule + |> Review.Test.expectNoErrors + , test "should not report links for different packages" <| + \() -> + """module A exposing (..) +{-| [link](https://package.elm-lang.org/packages/other-author/package/latest/Unknown/) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectNoErrors + ] + + +linksDependingOnExposition : Test +linksDependingOnExposition = + describe "Links depending on exposition" + [ test "should not report links from non-exposed modules to non-exposed modules" <| + \() -> + [ """module NotExposed exposing (a) +{-| [link](./AlsoNotExposed) +-} +a = 2 +""", """module AlsoNotExposed exposing (b) +b = 1 +""" ] + |> Review.Test.runOnModulesWithProjectData packageProject rule + |> Review.Test.expectNoErrors + , test "should not report links from non-exposed modules to exposed modules" <| + \() -> + """module NotExposed exposing (a) +{-| [link](./Exposed) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProject rule + |> Review.Test.expectNoErrors + , test "should not report links from exposed modules to exposed modules" <| + \() -> + """module Exposed2 exposing (a) +{-| [link](./Exposed) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProject rule + |> Review.Test.expectNoErrors + , test "should report links from exposed modules to non-exposed modules" <| + \() -> + [ """module Exposed2 exposing (a) +{-| [link](./NotExposed) +-} +a = 2 +""", """module NotExposed exposing (b) +b = 1 +""" ] + |> Review.Test.runOnModulesWithProjectData packageProject rule + |> Review.Test.expectErrorsForModules + [ ( "Exposed2" + , [ Review.Test.error + { message = "Link in public documentation points to non-exposed module" + , details = [ "Users will not be able to follow the link." ] + , under = "./NotExposed" + } + ] + ) + ] + , test "should not report links from non-exposed sections to non-exposed sections" <| + \() -> + [ """module Exposed2 exposing (b) +b = a + +{-| [link](./Exposed3#hidden) +-} +a = 1 +""", """module Exposed3 exposing (exposed) +exposed = 2 + +{-| +# Hidden +-} +a = 3 +""" ] + |> Review.Test.runOnModulesWithProjectData packageProject rule + |> Review.Test.expectNoErrors + , test "should not report links from non-exposed sections to exposed sections" <| + \() -> + [ """module Exposed2 exposing (b) +b = a + +{-| [link](./Exposed3#exposed) +-} +a = 1 +""", """module Exposed3 exposing (exposed) +exposed = 2 + +{-| +# Hidden +-} +a = 3 +""" ] + |> Review.Test.runOnModulesWithProjectData packageProject rule + |> Review.Test.expectNoErrors + , test "should report links from exposed sections to non-exposed sections" <| + \() -> + [ """module Exposed2 exposing (b) +{-| [link](./Exposed3#hidden) +-} +b = 1 +""", """module Exposed3 exposing (exposed) +exposed = 2 + +{-| +# Hidden +-} +a = 3 +""" ] + |> Review.Test.runOnModulesWithProjectData packageProject rule + |> Review.Test.expectErrorsForModules + [ ( "Exposed2" + , [ Review.Test.error + { message = "Link in public documentation points to non-exposed section" + , details = [ "Users will not be able to follow the link." ] + , under = "./Exposed3#hidden" + } + ] + ) + ] + , test "should report links from README to non-exposed modules" <| + \() -> + """module NotExposed exposing (a) +a = 1 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "[link](./NotExposed)" } packageProject) + rule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Link in public documentation points to non-exposed module" + , details = [ "Users will not be able to follow the link." ] + , under = "./NotExposed" + } + ] + , test "should not report links from README to exposed modules" <| + \() -> + """module NotExposed exposing (a) +a = 1 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "[link](./Exposed)" } packageProject) + rule + |> Review.Test.expectNoErrors + , test "should report links from README to non-exposed sections" <| + \() -> + """module Exposed2 exposing (a) +a = 1 +{-| +# Section +-} +b = 1 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "[link](./Exposed2#section)" } packageProject) + rule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Link in public documentation points to non-exposed section" + , details = [ "Users will not be able to follow the link." ] + , under = "./Exposed2#section" + } + ] + , test "should not report links from README to non-exposed sections in non-package projects" <| + \() -> + """module Exposed2 exposing (a) +a = 1 +{-| +# Section +-} +b = 1 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = "[link](./Exposed2#section)" } Project.new) + rule + |> Review.Test.expectNoErrors + ] + + +duplicateSectionsTests : Test +duplicateSectionsTests = + describe "Duplicate sections" + [ test "should report duplicate sections (declaration doc comment)" <| + \() -> + """module Exposed2 exposing (error, exposed) +{-| +# Error +-} +exposed = 1 + +error = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Duplicate section" + , details = [ "There are multiple sections that will result in the same id, meaning that links may point towards the wrong element." ] + , under = "# Error" + } + ] + , test "should report duplicate sections (module doc comment)" <| + \() -> + """module Exposed2 exposing (error) +{-| +# Error +-} + +error = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Duplicate section" + , details = [ "There are multiple sections that will result in the same id, meaning that links may point towards the wrong element." ] + , under = "# Error" + } + ] + , test "should report duplicate sections (all in declaration doc comments)" <| + \() -> + """module Exposed2 exposing (error) +{-| +# Some section + +# Some Section +-} + +error = 1 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Duplicate section" + , details = [ "There are multiple sections that will result in the same id, meaning that links may point towards the wrong element." ] + , under = "# Some Section" + } + ] + , test "should report duplicate sections in README" <| + \() -> + """module Exposed2 exposing (a) +a = 1 +""" + |> Review.Test.runWithProjectData + (Project.addReadme { path = "README.md", content = """ +# Some section + +# Some Section +""" } packageProject) + rule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Duplicate section" + , details = [ "There are multiple sections that will result in the same id, meaning that links may point towards the wrong element." ] + , under = "# Some Section" + } + ] + ] + + +unnecessaryLinksTests : Test +unnecessaryLinksTests = + describe "Unnecessary links" + [ test "should report links to #" <| + \() -> + """module A exposing (..) +{-| +[link](#) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link to empty section is unnecessary" + , details = [ "Links to # not followed by an id don't provide any value to the user. I suggest to either strip the # or remove the link." ] + , under = "#" + } + ] + , test "should report links to a module with #" <| + \() -> + """module A exposing (..) +{-| +[link](A#) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link to empty section is unnecessary" + , details = [ "Links to # not followed by an id don't provide any value to the user. I suggest to either strip the # or remove the link." ] + , under = "A#" + } + ] + ] + + +externalResourcesTests : Test +externalResourcesTests = + describe "External resources" + [ test "should not report links to external resources without a protocol when the project is not a package" <| + \() -> + """module A exposing (..) +{-| +[link](www.google.com) +![](./image.png) +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should not report links to external resources with a protocol" <| + \() -> + """module A exposing (..) +{-| +[link](https://foo.com) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectNoErrors + , test "should report links to an external resource without a protocol" <| + \() -> + """module A exposing (..) +{-| +[link](foo.com) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link to unknown resource without a protocol" + , details = + [ "I have trouble figuring out what kind of resource is linked here." + , "If it should link to a module, then they should be in the form 'Some-Module-Name'." + , "If it's a link to an external resource, they should start with a protocol, like `https://www.fruits.com`, otherwise the link will point to an unknown resource on package.elm-lang.org." + ] + , under = "foo.com" + } + ] + , test "should not report links to images with a protocol" <| + \() -> + """module A exposing (..) +{-| +[link](https://www.image.com/image.png) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectNoErrors + , test "should report links to images without a protocol" <| + \() -> + """module A exposing (..) +{-| +[link](./image.png) +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link to unknown resource without a protocol" + , details = + [ "I have trouble figuring out what kind of resource is linked here." + , "If it should link to a module, then they should be in the form 'Some-Module-Name'." + , "If it's a link to an external resource, they should start with a protocol, like `https://www.fruits.com`, otherwise the link will point to an unknown resource on package.elm-lang.org." + ] + , under = "./image.png" + } + ] + ] + + +linksWithAltTextTests : Test +linksWithAltTextTests = + describe "Alt text in links" + [ test "should report links to unknown modules in links with alt text after a slug" <| + \() -> + """module A exposing (a, b) +b = 1 + +{-| [link](Unknown#section "alt-text") +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to non-existing module Unknown" + , details = [ "This is a dead link." ] + , under = "Unknown#section" + } + ] + , test "should ignore alt text in links with a slug" <| + \() -> + """module A exposing (a, b) +b = 1 + +{-| [link](#b "alt-text") +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectNoErrors + , test "should report links to unknown modules in links with alt text without a slug" <| + \() -> + """module A exposing (a, b) +b = 1 + +{-| [link](Unknown "alt-text") +-} +a = 2 +""" + |> Review.Test.run rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to non-existing module Unknown" + , details = [ "This is a dead link." ] + , under = "Unknown" + } + ] + , test "should report absolute links to unknown modules in links with alt text with a slug" <| + \() -> + """module A exposing (a, b) +b = 1 + +{-| [link](https://package.elm-lang.org/packages/author/package/1.0.0/Unknown "alt-text") +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to non-existing module Unknown" + , details = [ "This is a dead link." ] + , under = "https://package.elm-lang.org/packages/author/package/1.0.0/Unknown" + } + ] + , test "should report absolute links to unknown modules in links with alt text without a slug" <| + \() -> + """module A exposing (a, b) +b = 1 + +{-| [link](https://package.elm-lang.org/packages/author/package/1.0.0/Unknown#section "alt-text") +-} +a = 2 +""" + |> Review.Test.runWithProjectData packageProjectWithoutFiles rule + |> Review.Test.expectErrors + [ Review.Test.error + { message = "Link points to non-existing module Unknown" + , details = [ "This is a dead link." ] + , under = "https://package.elm-lang.org/packages/author/package/1.0.0/Unknown#section" + } + ] + ] + + +packageProject : Project +packageProject = + Project.addModule + { path = "src/Exposed", source = "module Exposed exposing (exposed)\nexposed = 1" } + packageProjectWithoutFiles + + +packageProjectWithoutFiles : Project +packageProjectWithoutFiles = + case Json.Decode.decodeString Elm.Project.decoder elmJson of + Ok project -> + Project.new + |> Project.addElmJson + { path = "elm.json" + , raw = elmJson + , project = project + } + + Err err -> + Debug.todo ("Invalid elm.json supplied to test: " ++ Debug.toString err) + + +elmJson : String +elmJson = + """{ + "type": "package", + "name": "author/package", + "summary": "Summary", + "license": "BSD-3-Clause", + "version": "1.0.0", + "exposed-modules": [ + "Exposed", + "Exposed2", + "Exposed3" + ], + "elm-version": "0.19.0 <= v < 0.20.0", + "dependencies": { + "elm/core": "1.0.0 <= v < 2.0.0" + }, + "test-dependencies": {} +}""" diff --git a/tests/Docs/UpToDateReadmeLinks.elm b/tests/Docs/UpToDateReadmeLinks.elm new file mode 100644 index 00000000..0d922925 --- /dev/null +++ b/tests/Docs/UpToDateReadmeLinks.elm @@ -0,0 +1,183 @@ +module Docs.UpToDateReadmeLinks exposing (rule) + +{-| + +@docs rule + +-} + +import Docs.Utils.Link as Link exposing (Link) +import Elm.Package +import Elm.Project +import Elm.Syntax.Node exposing (Node(..)) +import Elm.Version +import Review.Fix as Fix +import Review.Rule as Rule exposing (Error, Rule) + + +{-| Reports links in the `README.md` that point to this project's package documentation on , +where the version is set to `latest` or a different version than the current version of the package. + +🔧 Running with `--fix` will automatically remove all the reported errors. + + config = + [ Docs.UpToDateReadmeLinks.rule + ] + +The problem with linking to `latest` is that if you release a new version later, +the users who read the README for the older version will be directed to a version +where the module/function/section you pointed to may not exist anymore. + +This rule ensures that you always use the correct version in all of your releases, +and that you do not forget to update the links. + +This rule provides automatic fixes, so you won't to do the tedious job of updating +the links yourself. + +**NOTE**: Just make sure to run tests between bumping the version of the package +and publishing the package. Otherwise the link for a given version could link to a previous one. + +**NOTE**: A similar rule would be useful for links inside the modules. I'll be working on that too! + + +## Try it out + +You can try this rule out by running the following command: + +```bash +elm-review --template jfmengels/elm-review-documentation/example --rules Docs.UpToDateReadmeLinks +``` + +-} +rule : Rule +rule = + Rule.newProjectRuleSchema "Docs.UpToDateReadmeLinks" initialProjectContext + |> Rule.withElmJsonProjectVisitor elmJsonVisitor + |> Rule.withReadmeProjectVisitor readmeVisitor + |> Rule.fromProjectRuleSchema + + +type alias ProjectContext = + Maybe + { projectName : String + , version : String + } + + +initialProjectContext : ProjectContext +initialProjectContext = + Nothing + + + +-- elm.json VISITOR + + +elmJsonVisitor : Maybe { a | project : Elm.Project.Project } -> ProjectContext -> ( List nothing, ProjectContext ) +elmJsonVisitor maybeProject _ = + case maybeProject |> Maybe.map .project of + Just (Elm.Project.Package pkg) -> + ( [] + , Just + { projectName = Elm.Package.toString pkg.name + , version = Elm.Version.toString pkg.version + } + ) + + _ -> + ( [], Nothing ) + + + +-- README VISITOR + + +readmeVisitor : Maybe { readmeKey : Rule.ReadmeKey, content : String } -> ProjectContext -> ( List (Error scope), ProjectContext ) +readmeVisitor maybeReadme maybeContext = + case ( maybeReadme, maybeContext ) of + ( Just { readmeKey, content }, Just context ) -> + ( reportErrorsForReadme context readmeKey content, maybeContext ) + + _ -> + ( [], maybeContext ) + + +reportErrorsForReadme : { projectName : String, version : String } -> Rule.ReadmeKey -> String -> List (Error scope) +reportErrorsForReadme context readmeKey content = + content + |> Link.findLinks 0 [] + |> List.concatMap (reportError context readmeKey) + + +reportError : { projectName : String, version : String } -> Rule.ReadmeKey -> Node Link -> List (Error scope) +reportError context readmeKey (Node range link) = + case link.file of + Link.ModuleTarget moduleName -> + [ Rule.errorForReadmeWithFix readmeKey + { message = "Found relative link to a module in README" + , details = + [ "Relative links to other modules from the README don't work when looking at the docs from GitHub or the likes." + , "I suggest to run elm-review --fix to change the link to an absolute link." + ] + } + range + [ Fix.replaceRangeBy range <| "https://package.elm-lang.org/packages/" ++ context.projectName ++ "/" ++ context.version ++ "/" ++ String.join "-" moduleName ++ formatSlug link.slug ] + ] + + Link.ReadmeTarget -> + if link.startsWithDotSlash then + [ Rule.errorForReadmeWithFix readmeKey + { message = "Found relative link from and to README" + , details = + [ "Links from and to README that start with \"./\" will not work on all places on GitHub or the likes." + , "I suggest to remove the leading \"./\"." + ] + } + range + (case link.slug of + Just slug -> + [ Fix.replaceRangeBy range ("#" ++ slug) ] + + Nothing -> + [] + ) + ] + + else + [] + + Link.PackagesTarget { name, version, subTarget } -> + if context.projectName == name && context.version /= version then + [ Rule.errorForReadmeWithFix readmeKey + { message = "Link does not point to the current version of the package" + , details = [ "I suggest to run elm-review --fix to get the correct link." ] + } + range + [ Fix.replaceRangeBy range <| "https://package.elm-lang.org/packages/" ++ name ++ "/" ++ context.version ++ "/" ++ formatSubTarget subTarget ++ formatSlug link.slug ] + ] + + else + [] + + Link.External _ -> + [] + + +formatSubTarget : Link.SubTarget -> String +formatSubTarget subTarget = + case subTarget of + Link.ModuleSubTarget moduleName -> + String.join "-" moduleName ++ "/" + + Link.ReadmeSubTarget -> + "" + + +formatSlug : Maybe String -> String +formatSlug maybeSlug = + case maybeSlug of + Just slug -> + "#" ++ slug + + Nothing -> + "" diff --git a/tests/Docs/UpToDateReadmeLinksTest.elm b/tests/Docs/UpToDateReadmeLinksTest.elm new file mode 100644 index 00000000..f4dfa9d1 --- /dev/null +++ b/tests/Docs/UpToDateReadmeLinksTest.elm @@ -0,0 +1,264 @@ +module Docs.UpToDateReadmeLinksTest exposing (all) + +import Docs.UpToDateReadmeLinks exposing (rule) +import Elm.Project +import Json.Decode as Decode +import Review.Project as Project exposing (Project) +import Review.Test exposing (ReviewResult) +import Test exposing (Test, describe, test) + + +testRule : Project -> ReviewResult +testRule project = + """module SomeModule exposing (a) +a = 1""" + |> Review.Test.runWithProjectData project rule + + +createElmJson : String -> { path : String, raw : String, project : Elm.Project.Project } +createElmJson rawElmJson = + case Decode.decodeString Elm.Project.decoder rawElmJson of + Ok elmJson -> + { path = "elm.json" + , raw = rawElmJson + , project = elmJson + } + + Err _ -> + Debug.todo "Invalid elm.json supplied to test" + + +packageElmJson : String -> String +packageElmJson name = + """ +{ + "type": "package", + "name": \"""" + ++ name + ++ """", + "summary": "Summary", + "license": "BSD-3-Clause", + "version": "1.2.3", + "exposed-modules": [ + "Exposed" + ], + "elm-version": "0.19.0 <= v < 0.20.0", + "dependencies": { + "elm/core": "1.0.0 <= v < 2.0.0" + }, + "test-dependencies": {} +}""" + + +message : String +message = + "Link does not point to the current version of the package" + + +details : List String +details = + [ "I suggest to run elm-review --fix to get the correct link." ] + + +readmeWithLink : String -> String +readmeWithLink link = + """ +# My project + + - [my project's thing](""" ++ link ++ """) +Don't report: + - [this](https://package.elm-lang.org/packages/author/other-package/latest/Module-Name) + - [this](https://package.elm-lang.org/packages/author/other-package/1.2.2/Module-Name) + - [this](https://package.elm-lang.org/packages/other-author/package/latest/Module-Name) + - [this](https://package.elm-lang.org/packages/other-author/package/1.2.4/Module-Name) +""" + + +all : Test +all = + describe "Docs.UpToDateReadmeLinks" + [ test "should not report an error if there is no elm.json file" <| + \() -> + Project.new + |> addReadme "https://package.elm-lang.org/packages/author/package/1.2.4/Module-Name" + |> testRule + |> Review.Test.expectNoErrors + , test "should not report an error if there is no README file" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "author/package") + |> testRule + |> Review.Test.expectNoErrors + , test "should not report an error if all the links point to the current project use the correct version" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "author/package") + |> addReadme "https://package.elm-lang.org/packages/author/package/1.2.3/Module-Name" + |> testRule + |> Review.Test.expectNoErrors + , test "should report an error if a link points to a different version" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "author/package") + |> addReadme "https://package.elm-lang.org/packages/author/package/1.2.4/Module-Name" + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = message + , details = details + , under = "https://package.elm-lang.org/packages/author/package/1.2.4/Module-Name" + } + |> Review.Test.whenFixed (readmeWithLink "https://package.elm-lang.org/packages/author/package/1.2.3/Module-Name/") + ] + , test "should report errors for multiple links on the same line" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "author/package") + |> Project.addReadme { path = "README.md", content = """ +[link1](https://package.elm-lang.org/packages/author/package/1.2.4/A) [link2](https://package.elm-lang.org/packages/author/package/1.2.4/B) +""" } + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = message + , details = details + , under = "https://package.elm-lang.org/packages/author/package/1.2.4/A" + } + |> Review.Test.whenFixed """ +[link1](https://package.elm-lang.org/packages/author/package/1.2.3/A/) [link2](https://package.elm-lang.org/packages/author/package/1.2.4/B) +""" + , Review.Test.error + { message = message + , details = details + , under = "https://package.elm-lang.org/packages/author/package/1.2.4/B" + } + |> Review.Test.whenFixed """ +[link1](https://package.elm-lang.org/packages/author/package/1.2.4/A) [link2](https://package.elm-lang.org/packages/author/package/1.2.3/B/) +""" + ] + , test "should report an error if a link points to latest" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "author/package") + |> addReadme "https://package.elm-lang.org/packages/author/package/latest/Module-Name" + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = message + , details = details + , under = "https://package.elm-lang.org/packages/author/package/latest/Module-Name" + } + |> Review.Test.whenFixed (readmeWithLink "https://package.elm-lang.org/packages/author/package/1.2.3/Module-Name/") + ] + , test "should report an error even if the author or package name contains a dash or digit" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "au-tho5r/pack-age1") + |> addReadme "https://package.elm-lang.org/packages/au-tho5r/pack-age1/latest/Module-Name" + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = message + , details = details + , under = "https://package.elm-lang.org/packages/au-tho5r/pack-age1/latest/Module-Name" + } + |> Review.Test.whenFixed (readmeWithLink "https://package.elm-lang.org/packages/au-tho5r/pack-age1/1.2.3/Module-Name/") + ] + , test "should report an error if the link is relative" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "au-tho5r/pack-age1") + |> addReadme "Some-Module-Name" + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Found relative link to a module in README" + , details = + [ "Relative links to other modules from the README don't work when looking at the docs from GitHub or the likes." + , "I suggest to run elm-review --fix to change the link to an absolute link." + ] + , under = "Some-Module-Name" + } + |> Review.Test.whenFixed (readmeWithLink "https://package.elm-lang.org/packages/au-tho5r/pack-age1/1.2.3/Some-Module-Name") + ] + , test "should report an error if the link is relative with a section" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "au-tho5r/pack-age1") + |> addReadme "Some-Module-Name#section" + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Found relative link to a module in README" + , details = + [ "Relative links to other modules from the README don't work when looking at the docs from GitHub or the likes." + , "I suggest to run elm-review --fix to change the link to an absolute link." + ] + , under = "Some-Module-Name#section" + } + |> Review.Test.whenFixed (readmeWithLink "https://package.elm-lang.org/packages/au-tho5r/pack-age1/1.2.3/Some-Module-Name#section") + ] + , test "should not report an error if the link is relative to the README" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "au-tho5r/pack-age1") + |> addReadme "#section" + |> testRule + |> Review.Test.expectNoErrors + , test "should report an error if the link is relative to the README but starts with ./" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "au-tho5r/pack-age1") + |> addReadme "./#section" + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Found relative link from and to README" + , details = + [ "Links from and to README that start with \"./\" will not work on all places on GitHub or the likes." + , "I suggest to remove the leading \"./\"." + ] + , under = "./#section" + } + |> Review.Test.whenFixed (readmeWithLink "#section") + ] + , test "should report an error but not provide a fix if the link is exactly ./" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "au-tho5r/pack-age1") + |> addReadme "./" + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Found relative link from and to README" + , details = + [ "Links from and to README that start with \"./\" will not work on all places on GitHub or the likes." + , "I suggest to remove the leading \"./\"." + ] + , under = "./" + } + ] + , test "should report at the correct location when there are unicode characters in the line" <| + \() -> + Project.new + |> Project.addElmJson (createElmJson <| packageElmJson "au-tho5r/pack-age1") + |> Project.addReadme { path = "README.md", content = """ +[🔧 `Rule.Name`](https://package.elm-lang.org/packages/au-tho5r/pack-age1/latest/) +""" } + |> testRule + |> Review.Test.expectErrorsForReadme + [ Review.Test.error + { message = "Link does not point to the current version of the package" + , details = details + , under = "https://package.elm-lang.org/packages/au-tho5r/pack-age1/latest/" + } + |> Review.Test.whenFixed """ +[🔧 `Rule.Name`](https://package.elm-lang.org/packages/au-tho5r/pack-age1/1.2.3/) +""" + ] + ] + + +addReadme : String -> Project -> Project +addReadme contents = + Project.addReadme { path = "README.md", content = readmeWithLink contents } diff --git a/tests/Docs/Utils/ExposedFromProject.elm b/tests/Docs/Utils/ExposedFromProject.elm new file mode 100644 index 00000000..d1b39edb --- /dev/null +++ b/tests/Docs/Utils/ExposedFromProject.elm @@ -0,0 +1,25 @@ +module Docs.Utils.ExposedFromProject exposing (exposedModules) + +import Elm.Module +import Elm.Project +import Set exposing (Set) + + +exposedModules : Elm.Project.Project -> Set String +exposedModules project = + case project of + Elm.Project.Package package -> + case package.exposed of + Elm.Project.ExposedList list -> + list + |> List.map Elm.Module.toString + |> Set.fromList + + Elm.Project.ExposedDict list -> + list + |> List.concatMap Tuple.second + |> List.map Elm.Module.toString + |> Set.fromList + + Elm.Project.Application _ -> + Set.empty diff --git a/tests/Docs/Utils/Link.elm b/tests/Docs/Utils/Link.elm new file mode 100644 index 00000000..6347c739 --- /dev/null +++ b/tests/Docs/Utils/Link.elm @@ -0,0 +1,335 @@ +module Docs.Utils.Link exposing + ( FileTarget(..) + , Link + , SubTarget(..) + , findLinks + ) + +import Elm.Syntax.ModuleName exposing (ModuleName) +import Elm.Syntax.Node exposing (Node(..)) +import Elm.Syntax.Range exposing (Location, Range) +import Parser exposing ((|.), (|=), Parser) +import Regex exposing (Regex) + + +addOffset : Int -> Range -> Range +addOffset lineNumber { start, end } = + { start = addLineNumber lineNumber start + , end = addLineNumber lineNumber end + } + + +addLineNumber : Int -> Location -> Location +addLineNumber lineNumber { row, column } = + { row = lineNumber + row + , column = column + 1 + } + + +type alias Link = + { file : FileTarget + , startsWithDotSlash : Bool + , slug : Maybe String + } + + +type FileTarget + = ModuleTarget ModuleName + | ReadmeTarget + | PackagesTarget { name : String, version : String, subTarget : SubTarget } + | External String + + +type SubTarget + = ModuleSubTarget ModuleName + | ReadmeSubTarget + + +idParser : Char -> Parser String +idParser endChar = + Parser.succeed () + |. Parser.chompWhile (\c -> c /= endChar && c /= ' ') + |> Parser.getChompedString + + +onlyModuleNameParser : Parser ModuleName +onlyModuleNameParser = + Parser.succeed identity + |= moduleNameParser + |. Parser.end + + +moduleNameParser : Parser ModuleName +moduleNameParser = + manySeparated + { by = "-" + , item = moduleNameSegmentParser + } + + +moduleNameSegmentParser : Parser String +moduleNameSegmentParser = + Parser.succeed () + |. Parser.chompIf (\c -> Char.isUpper c) + |. Parser.chompWhile (\c -> Char.isAlphaNum c) + |> Parser.getChompedString + + +findLinks : Int -> ModuleName -> String -> List (Node Link) +findLinks row moduleName string = + string + |> String.lines + |> List.indexedMap + (\lineNumber lineContent -> + lineContent + |> Parser.run (findParser (linkParser (lineNumber + row) moduleName)) + |> Result.withDefault [] + |> List.filterMap identity + |> List.indexedMap + (\index (Node { start, end } link) -> + Node + { start = { row = start.row, column = start.column - (index * 2) } + , end = { row = end.row, column = end.column - (index * 2) } + } + link + ) + ) + |> List.concat + + +linkParser : Int -> ModuleName -> Parser (Maybe (Node Link)) +linkParser row moduleName = + Parser.succeed identity + |= Parser.getCol + |. bracketsParser + |> Parser.andThen + (\col -> + if col == 1 then + Parser.oneOf + [ inlineLinkParser + |> Parser.map Just + , referenceLinkParser + |> Parser.map Just + , Parser.succeed Nothing + ] + + else + Parser.oneOf + [ Parser.map Just inlineLinkParser + , Parser.succeed Nothing + ] + ) + |> Parser.map + (Maybe.map + (\(Node range link) -> + Node (addOffset row range) (normalizeModuleName moduleName link) + ) + ) + + +normalizeModuleName : ModuleName -> Link -> Link +normalizeModuleName currentModuleName link = + case link.file of + ModuleTarget [] -> + let + file : FileTarget + file = + if List.isEmpty currentModuleName then + ReadmeTarget + + else + ModuleTarget currentModuleName + in + { link | file = file } + + ModuleTarget _ -> + link + + ReadmeTarget -> + link + + PackagesTarget _ -> + link + + External _ -> + link + + +{-| Parses things like: + + This is a [link](#Link). + +-} +inlineLinkParser : Parser (Node Link) +inlineLinkParser = + Parser.succeed + (\( startRow, startCol ) link ( endRow, endCol ) -> + Node + { start = { row = startRow, column = startCol - 2 } + , end = { row = endRow, column = endCol - 2 } + } + link + ) + |. Parser.symbol "(" + |= Parser.getPosition + |= pathParser ')' + |= Parser.getPosition + |. Parser.chompUntil ")" + |. Parser.symbol ")" + + +{-| Parses things like: + + [link]: #Link + +-} +referenceLinkParser : Parser (Node Link) +referenceLinkParser = + Parser.succeed + (\( startRow, startCol ) link ( endRow, endCol ) -> + Node + { start = { row = startRow, column = startCol - 2 } + , end = { row = endRow, column = endCol - 2 } + } + link + ) + |. Parser.symbol ":" + |. Parser.spaces + |= Parser.getPosition + |= pathParser '\n' + |= Parser.getPosition + + +pathParser : Char -> Parser Link +pathParser endChar = + Parser.oneOf + [ Parser.succeed + (\section -> + { file = ModuleTarget [], startsWithDotSlash = False, slug = Just section } + ) + |. Parser.symbol "#" + |= idParser endChar + , Parser.succeed (\startsWithDotSlash file slug -> { file = file, startsWithDotSlash = startsWithDotSlash, slug = slug }) + |= ignoreDotSlash + |= parseModuleName + |= optionalSectionParser endChar + ] + + +optionalSectionParser : Char -> Parser (Maybe String) +optionalSectionParser endChar = + Parser.oneOf + [ Parser.succeed Just + |. Parser.symbol "#" + |= idParser endChar + , Parser.succeed Nothing + ] + + +parseModuleName : Parser FileTarget +parseModuleName = + Parser.succeed () + |. Parser.chompWhile (\c -> c /= '#' && c /= ')' && c /= ' ') + |> Parser.getChompedString + |> Parser.map + (\linkTarget -> + if linkTarget == "" then + ReadmeTarget + + else + case Parser.run onlyModuleNameParser linkTarget of + Ok moduleName -> + ModuleTarget moduleName + + Err _ -> + case Regex.find linkRegex linkTarget |> List.head |> Maybe.andThen parseSubTarget of + Just fileTarget -> + fileTarget + + Nothing -> + External linkTarget + ) + + +parseSubTarget : Regex.Match -> Maybe FileTarget +parseSubTarget match = + case match.submatches of + (Just authorAndPackage) :: (Just linkVersion) :: _ :: rest :: [] -> + let + subTarget : SubTarget + subTarget = + case rest of + Just nonemptyModuleName -> + nonemptyModuleName + |> String.replace "/" "" + |> String.split "-" + |> ModuleSubTarget + + Nothing -> + ReadmeSubTarget + in + Just (PackagesTarget { name = authorAndPackage, version = linkVersion, subTarget = subTarget }) + + _ -> + Nothing + + +linkRegex : Regex +linkRegex = + Regex.fromString "https://package\\.elm-lang\\.org/packages/([\\w-]+/[\\w-]+)/(latest|\\w+\\.\\w+\\.\\w+)(/(.*))?" + |> Maybe.withDefault Regex.never + + +ignoreDotSlash : Parser Bool +ignoreDotSlash = + Parser.oneOf + [ Parser.symbol "." + |. Parser.symbol "/" + |> Parser.map (\_ -> True) + , Parser.succeed False + ] + + +bracketsParser : Parser () +bracketsParser = + Parser.succeed identity + |. Parser.symbol "[" + |. Parser.spaces + |= Parser.chompUntil "]" + |. Parser.spaces + |. Parser.symbol "]" + + +findParser : Parser a -> Parser (List a) +findParser parser = + Parser.loop [] + (\parsed -> + Parser.oneOf + [ Parser.succeed (\p -> p :: parsed) + |= parser + |> Parser.map Parser.Loop + , Parser.succeed parsed + |. Parser.chompIf (\_ -> True) + |> Parser.map Parser.Loop + , Parser.end + |> Parser.map (\() -> Parser.Done (List.reverse parsed)) + ] + ) + + +{-| 0 or more things directly separated by a string like "go-gi-ga". +-} +manySeparated : + { by : String + , item : Parser between + } + -> Parser (List between) +manySeparated { by, item } = + Parser.sequence + { start = "" + , separator = by + , end = "" + , spaces = Parser.symbol "" + , item = item + , trailing = Parser.Forbidden + } diff --git a/tests/Docs/Utils/Slug.elm b/tests/Docs/Utils/Slug.elm new file mode 100644 index 00000000..0fdbb361 --- /dev/null +++ b/tests/Docs/Utils/Slug.elm @@ -0,0 +1,41 @@ +module Docs.Utils.Slug exposing (toSlug) + +import Regex exposing (Regex) + + +toSlug : String -> String +toSlug string = + string + |> String.toLower + |> Regex.replace specialsToStrip (\_ -> "") + |> Regex.replace specialsToReplaceByDash (\_ -> "-") + |> Regex.replace specialsToReplaceByUnderscore (\_ -> "_") + |> Regex.replace multipleDashes (\_ -> "-") + + +specialsToReplaceByDash : Regex +specialsToReplaceByDash = + "([.() \\[\\]`?]|\\\\)" + |> Regex.fromString + |> Maybe.withDefault Regex.never + + +specialsToReplaceByUnderscore : Regex +specialsToReplaceByUnderscore = + "\\*" + |> Regex.fromString + |> Maybe.withDefault Regex.never + + +specialsToStrip : Regex +specialsToStrip = + "[~$]|\\[`|`\\]" + |> Regex.fromString + |> Maybe.withDefault Regex.never + + +multipleDashes : Regex +multipleDashes = + "-+" + |> Regex.fromString + |> Maybe.withDefault Regex.never diff --git a/tests/Docs/Utils/SlugTest.elm b/tests/Docs/Utils/SlugTest.elm new file mode 100644 index 00000000..7104789b --- /dev/null +++ b/tests/Docs/Utils/SlugTest.elm @@ -0,0 +1,36 @@ +module Docs.Utils.SlugTest exposing (all) + +import Docs.Utils.Slug as Slug +import Expect +import Test exposing (Test, describe, test) + + +all : Test +all = + describe "Id slugs" + [ test "should slugify single-word section" <| + \() -> + "Section" + |> Slug.toSlug + |> Expect.equal "section" + , test "should slugify section with spaces" <| + \() -> + "Some Section" + |> Slug.toSlug + |> Expect.equal "some-section" + , test "should slugify section with back-ticks" <| + \() -> + "`section`" + |> Slug.toSlug + |> Expect.equal "-section-" + , test "should slugify section with question mark" <| + \() -> + "Section?" + |> Slug.toSlug + |> Expect.equal "section-" + , test "should slugify complex example" <| + \() -> + "Section *with* ~some~ _spaces_ and\\_ $thi.ngs . [`links`](foo)" + |> Slug.toSlug + |> Expect.equal "section-_with_-some-_spaces_-and-_-thi-ngs-links-foo-" + ] diff --git a/tests/Documentation/ReadmeLinksPointToCurrentVersion.elm b/tests/Documentation/ReadmeLinksPointToCurrentVersion.elm deleted file mode 100644 index bc403cbc..00000000 --- a/tests/Documentation/ReadmeLinksPointToCurrentVersion.elm +++ /dev/null @@ -1,147 +0,0 @@ -module Documentation.ReadmeLinksPointToCurrentVersion exposing (rule) - -{-| - -@docs rule - --} - -import Elm.Package -import Elm.Project -import Elm.Syntax.Range exposing (Range) -import Elm.Version -import Regex exposing (Regex) -import Review.Fix as Fix -import Review.Rule as Rule exposing (Error, Rule) - - -{-| Reports links in the `README.md` that point to this project's package documentation on , -where the version is set to `latest` or a different version than the current version of the package. - -The problem with linking to `latest` is that if you release a new version later, -the users who read the README for the older version will be directed to a version -where the module/function/section you pointed to may not exist anymore. - -This rule ensures that you always use the correct version in all of your releases, -and that you do not forget to update the links. - -This rule provides automatic fixes, so you won't to do the tedious job of updating -the links yourself. - -**NOTE**: Just make sure to run tests between bumping the version of the package -and publishing the package. Otherwise the link for a given version could link to a previous one. - -**NOTE**: A similar rule would be useful for links inside the modules. I'll be working on that too! - - -## Try it out - -You can try this rule out by running the following command: - -```bash -elm-review --template jfmengels/elm-review-documentation/example --rules NoUselessSubscriptions -``` - --} -rule : Rule -rule = - Rule.newProjectRuleSchema "ReadmeLinksPointToCurrentVersion" initialProjectContext - |> Rule.withElmJsonProjectVisitor elmJsonVisitor - |> Rule.withReadmeProjectVisitor readmeVisitor - |> Rule.fromProjectRuleSchema - - -type alias ProjectContext = - Maybe - { projectName : String - , version : String - } - - -initialProjectContext : ProjectContext -initialProjectContext = - Nothing - - - --- elm.json VISITOR - - -elmJsonVisitor : Maybe { a | project : Elm.Project.Project } -> ProjectContext -> ( List nothing, ProjectContext ) -elmJsonVisitor maybeProject _ = - case maybeProject |> Maybe.map .project of - Just (Elm.Project.Package pkg) -> - ( [] - , Just - { projectName = Elm.Package.toString pkg.name - , version = Elm.Version.toString pkg.version - } - ) - - _ -> - ( [], Nothing ) - - - --- README VISITOR - - -readmeVisitor : Maybe { readmeKey : Rule.ReadmeKey, content : String } -> ProjectContext -> ( List (Error scope), ProjectContext ) -readmeVisitor maybeReadme maybeContext = - case ( maybeReadme, maybeContext ) of - ( Just { readmeKey, content }, Just context ) -> - ( findRangeForSubstring context readmeKey content, maybeContext ) - - _ -> - ( [], maybeContext ) - - -linkRegex : Regex -linkRegex = - Regex.fromString "]\\(https://package\\.elm-lang\\.org/packages/([\\w-]+/[\\w-]+)/(\\w+(\\.\\w+\\.\\w+)?)(.*)\\)" - |> Maybe.withDefault Regex.never - - -findRangeForSubstring : { projectName : String, version : String } -> Rule.ReadmeKey -> String -> List (Error scope) -findRangeForSubstring context readmeKey content = - content - |> String.lines - |> List.indexedMap Tuple.pair - |> List.concatMap - (\( row, line ) -> - Regex.find linkRegex line - |> List.filterMap (notAMatch context readmeKey row) - ) - - -notAMatch : { projectName : String, version : String } -> Rule.ReadmeKey -> Int -> Regex.Match -> Maybe (Error scope) -notAMatch { projectName, version } readmeKey row match = - case match.submatches of - (Just authorAndPackage) :: (Just linkVersion) :: _ :: rest :: [] -> - if authorAndPackage == projectName && linkVersion /= version then - let - range : Range - range = - { start = - { row = row + 1 - , column = match.index + 3 - } - , end = - { row = row + 1 - , column = match.index + String.length match.match - } - } - in - Rule.errorForReadmeWithFix readmeKey - { message = "Link does not point to the current version of the package" - , details = [ "I suggest to run elm-review --fix to get the correct links." ] - } - range - [ Fix.replaceRangeBy range <| "https://package.elm-lang.org/packages/" ++ projectName ++ "/" ++ version ++ Maybe.withDefault "" rest ] - |> Just - - else - Nothing - - _ -> - Nothing diff --git a/tests/Documentation/ReadmeLinksPointToCurrentVersionTest.elm b/tests/Documentation/ReadmeLinksPointToCurrentVersionTest.elm deleted file mode 100644 index 719c9dbd..00000000 --- a/tests/Documentation/ReadmeLinksPointToCurrentVersionTest.elm +++ /dev/null @@ -1,141 +0,0 @@ -module Documentation.ReadmeLinksPointToCurrentVersionTest exposing (all) - -import Documentation.ReadmeLinksPointToCurrentVersion exposing (rule) -import Elm.Project -import Json.Decode as Decode -import Review.Project as Project exposing (Project) -import Review.Test exposing (ReviewResult) -import Test exposing (Test, describe, test) - - -testRule : Project -> ReviewResult -testRule project = - """module SomeModule exposing (a) -a = 1""" - |> Review.Test.runWithProjectData project rule - - -createElmJson : String -> { path : String, raw : String, project : Elm.Project.Project } -createElmJson rawElmJson = - case Decode.decodeString Elm.Project.decoder rawElmJson of - Ok elmJson -> - { path = "elm.json" - , raw = rawElmJson - , project = elmJson - } - - Err _ -> - Debug.todo "Invalid elm.json supplied to test" - - -packageElmJson : String -> String -packageElmJson name = - """ -{ - "type": "package", - "name": \"""" - ++ name - ++ """", - "summary": "Summary", - "license": "BSD-3-Clause", - "version": "1.2.3", - "exposed-modules": [ - "Exposed" - ], - "elm-version": "0.19.0 <= v < 0.20.0", - "dependencies": { - "elm/core": "1.0.0 <= v < 2.0.0" - }, - "test-dependencies": {} -}""" - - -message : String -message = - "Link does not point to the current version of the package" - - -details : List String -details = - [ "I suggest to run elm-review --fix to get the correct links." ] - - -readmeWithLink : String -> String -readmeWithLink link = - """ -# My project - - - [my project's thing](""" ++ link ++ """) -Don't report: - - [this](https://package.elm-lang.org/packages/author/other-package/latest/Module-Name) - - [this](https://package.elm-lang.org/packages/author/other-package/1.2.2/Module-Name) - - [this](https://package.elm-lang.org/packages/other-author/package/latest/Module-Name) - - [this](https://package.elm-lang.org/packages/other-author/package/1.2.4/Module-Name) -""" - - -all : Test -all = - describe "ReadmeLinksPointToCurrentVersion" - [ test "should not report an error if there is no elm.json file" <| - \() -> - Project.new - |> Project.addReadme { path = "README.md", content = readmeWithLink "https://package.elm-lang.org/packages/author/package/1.2.4/Module-Name" } - |> testRule - |> Review.Test.expectNoErrors - , test "should not report an error if there is no README file" <| - \() -> - Project.new - |> Project.addElmJson (createElmJson <| packageElmJson "author/package") - |> testRule - |> Review.Test.expectNoErrors - , test "should not report an error if all the links point to the current project use the correct version" <| - \() -> - Project.new - |> Project.addElmJson (createElmJson <| packageElmJson "author/package") - |> Project.addReadme { path = "README.md", content = readmeWithLink "https://package.elm-lang.org/packages/author/package/1.2.3/Module-Name" } - |> testRule - |> Review.Test.expectNoErrors - , test "should report an error if a link points to a different version" <| - \() -> - Project.new - |> Project.addElmJson (createElmJson <| packageElmJson "author/package") - |> Project.addReadme { path = "README.md", content = readmeWithLink "https://package.elm-lang.org/packages/author/package/1.2.4/Module-Name" } - |> testRule - |> Review.Test.expectErrorsForReadme - [ Review.Test.error - { message = message - , details = details - , under = "https://package.elm-lang.org/packages/author/package/1.2.4/Module-Name" - } - |> Review.Test.whenFixed (readmeWithLink "https://package.elm-lang.org/packages/author/package/1.2.3/Module-Name") - ] - , test "should report an error if a link points to latest" <| - \() -> - Project.new - |> Project.addElmJson (createElmJson <| packageElmJson "author/package") - |> Project.addReadme { path = "README.md", content = readmeWithLink "https://package.elm-lang.org/packages/author/package/latest/Module-Name" } - |> testRule - |> Review.Test.expectErrorsForReadme - [ Review.Test.error - { message = message - , details = details - , under = "https://package.elm-lang.org/packages/author/package/latest/Module-Name" - } - |> Review.Test.whenFixed (readmeWithLink "https://package.elm-lang.org/packages/author/package/1.2.3/Module-Name") - ] - , test "should report an error even if the author or package name contains a dash or digit" <| - \() -> - Project.new - |> Project.addElmJson (createElmJson <| packageElmJson "au-tho5r/pack-age1") - |> Project.addReadme { path = "README.md", content = readmeWithLink "https://package.elm-lang.org/packages/au-tho5r/pack-age1/latest/Module-Name" } - |> testRule - |> Review.Test.expectErrorsForReadme - [ Review.Test.error - { message = message - , details = details - , under = "https://package.elm-lang.org/packages/au-tho5r/pack-age1/latest/Module-Name" - } - |> Review.Test.whenFixed (readmeWithLink "https://package.elm-lang.org/packages/au-tho5r/pack-age1/1.2.3/Module-Name") - ] - ]