Fix module name lookup table giving a wrong result

Let destructuring variables that shadowed imports were not correctly
registered, and looking them up could lead to incorrectly point to
the imported module that was shadowed.

Thanks @matzko for discovering this.

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+## [2.13.2] - 2024-04-09
+
+Fixed an issue where the result of a module name lookup for let variables was sometimes incorrect.
+
 ## [2.13.1] - 2023-06-17
 
 Fixed an issue where the module name lookup table would yield an incorrect result. [#159](https://github.com/jfmengels/elm-review/pull/159)

src/Review/ModuleNameLookupTable/Compute.elm

@@ -1215,7 +1215,7 @@ popScopeExit node context =
 expressionEnterVisitor : Node Expression -> Context -> Context
 expressionEnterVisitor node context =
     case Node.value node of
-        Expression.LetExpression { declarations } ->
+        Expression.LetExpression letExpression ->
             let
                 newContext : Context
                 newContext =
@@ -1246,11 +1246,11 @@ expressionEnterVisitor node context =
                                         in
                                         NonEmpty.mapHead (\scope -> { scope | cases = ( expression, names ) :: scope.cases }) withLetVariable
 
-                                Expression.LetDestructuring _ _ ->
-                                    scopes
+                                Expression.LetDestructuring pattern _ ->
+                                    NonEmpty.mapHead (\scope -> { scope | names = collectNamesFromPattern LetVariable [ pattern ] scope.names }) scopes
                         )
                         (NonEmpty.cons emptyScope context.scopes)
-                        declarations
+                        letExpression.declarations
                         |> updateScope context
 
                 lookupTable : ModuleNameLookupTable
@@ -1280,7 +1280,7 @@ expressionEnterVisitor node context =
                                     collectModuleNamesFromPattern newContext [ pattern ] acc
                         )
                         newContext.lookupTable
-                        declarations
+                        letExpression.declarations
             in
             { newContext | lookupTable = lookupTable }
 

tests/ModuleNameLookupTableTest.elm

@@ -103,6 +103,7 @@ a = localValue
  ("x" </> "y")
  (|=)
  ("pars" |= "er")
+ (let (exposedElement, _) = 1 in exposedElement)
 b = case () of
   VariantA -> ()
   (ExposesEverything.VariantA as foo) -> foo
@@ -167,6 +168,7 @@ Cmd.none -> Platform.Cmd.none
 <nothing>.</> -> Url.Parser.</>
 <nothing>.|= -> Parser.Advanced.|=
 <nothing>.|= -> Parser.Advanced.|=
+<nothing>.exposedElement -> <nothing>.exposedElement
 <nothing>.VariantA -> ExposesEverything.VariantA
 ExposesEverything.VariantA -> ExposesEverything.VariantA
 ExposesEverythingAlias.VariantA -> ExposesEverything.VariantA
@@ -316,6 +318,7 @@ a = localValue
  ("x" </> "y")
  (|=)
  ("pars" |= "er")
+ (let (exposedElement, _) = 1 in exposedElement)
 b = case () of
   VariantA -> ()
   (ExposesEverything.VariantA as foo) -> foo
@@ -378,6 +381,7 @@ Cmd.none -> Platform.Cmd.none
 <nothing>.</> -> Url.Parser.</>
 <nothing>.|= -> Parser.Advanced.|=
 <nothing>.|= -> Parser.Advanced.|=
+<nothing>.exposedElement -> Abc.Xyz.exposedElement
 <nothing>.VariantA -> ExposesEverything.VariantA
 ExposesEverything.VariantA -> ExposesEverything.VariantA
 ExposesEverythingAlias.VariantA -> ExposesEverything.VariantA
@@ -698,6 +702,15 @@ collectPatterns lookupFunction context node =
         Pattern.VarPattern _ ->
             []
 
+        Pattern.TuplePattern subPatterns ->
+            List.concatMap (collectPatterns lookupFunction context) subPatterns
+
+        Pattern.RecordPattern subPatterns ->
+            List.map Node.value subPatterns
+
+        Pattern.AllPattern ->
+            []
+
         _ ->
             Debug.todo ("Other patterns in case expressions are not handled: " ++ Debug.toString node)