jfmengels / elm-review

Analyzes Elm projects, to help find mistakes before your users find them.
https://package.elm-lang.org/packages/jfmengels/elm-review/latest/
Other
252 stars 13 forks source link

Add `Review.Rule.with...Type(Annotation)...Visitor`s #122

Closed lue-bird closed 2 years ago

lue-bird commented 2 years ago

Including TypeAnnotation visitors would be useful to for example forbid tuple types, specific named types, ...:

(I'm in favor of Type as opposed to annotation which would imply top-level, not stepping deep inside; Only downside I can see is an unlikely confusion with a tagged union declaration)

It is currently quite a lot of code and therefore error-prone. Maybe the code I wrote can help with the implementation just a little :shrug::

import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation, LetDeclaration)
import Elm.Syntax.Declaration as TypeAnnotation exposing (Declaration)
import Elm.Syntax.Node exposing (Node(..))

declarationLetNamedThings : LetDeclaration -> ...
declarationLetNamedThings =
    \declarationLet ->
        case declarationLet of
            Expression.LetFunction { declaration, signature } ->
                case signature of
                    Nothing ->
                        []

                    Just (Node _ { typeAnnotation }) ->
                        typeAnnotation |> ...

            Expression.LetDestructuring destructuringPattern _ ->
                destructuringPattern |> ...

reviewDeclarationModuleScope : Declaration -> ...
reviewDeclarationModuleScope =
    \declaration ->
        case declaration of
            Declaration.FunctionDeclaration function ->
                case function.signature of
                    Just (Node _ { typeAnnotation }) ->
                        typeAnnotation |> ...

                    Nothing ->
                        []

            Declaration.AliasDeclaration { typeAnnotation } ->
                typeAnnotation |> ...

            Declaration.CustomTypeDeclaration { constructors } ->
                constructors
                    |> List.concatMap
                        (\(Node _ { arguments }) ->
                            arguments
                                |> List.concatMap ...
                        )

            Declaration.PortDeclaration { typeAnnotation } ->
                typeAnnotation |> ...

            -- not possible in modules outside elm/...
            Declaration.InfixDeclaration { function } ->
                []

            -- Such a declaration doesn't exist in elm.
            Declaration.Destructuring _ _ ->
                []

{-| All types that are itself part of the whole type.

Doesn't collect type literals, just looks one step deeper into the whole type.

-}
typeChildren : TypeAnnotation -> List (Node TypeAnnotation)
typeChildren =
    \type_ ->
        case type_ of
            Type.Unit ->
                []

            Type.GenericType _ ->
                []

            Type.FunctionTypeAnnotation argument result ->
                [ argument, result ]

            Type.Tupled parts ->
                parts

            Type.Typed _ arguments ->
                arguments

            Type.Record fields ->
                fields |> List.map (\(Node _ ( _, value )) -> value)

            Type.GenericRecord _ (Node _ fields) ->
                fields
                    |> List.map (\(Node _ ( _, value )) -> value)
jfmengels commented 2 years ago

I think the proposal in my comment in https://github.com/jfmengels/elm-review/issues/121#issuecomment-1129284921 would be a better way to solve the itch.

jfmengels commented 2 years ago

Closing this issue seeing as @lue-bird also agrees that my proposal would fit better.