Improve error message for out-of-scope variables + VTAwip/T13834

As #13834 and #17150 report, we get a TERRIBLE error message when you
have an out of scope variable applied in a visible type application:
     (outOfScope @Int True)

This very simple patch improves matters.

See TcExpr Note [VTA for out-of-scope functions]

6 files changed, 55 insertions, 13 deletions

diff --git a/compiler/typecheck/TcExpr.hs b/compiler/typecheck/TcExpr.hs
index df735d23d6..1387e89089 100644
--- a/compiler/typecheck/TcExpr.hs
+++ b/compiler/typecheck/TcExpr.hs
@@ -1104,6 +1104,10 @@ isHsValArg (HsValArg {})  = True
 isHsValArg (HsTypeArg {}) = False
 isHsValArg (HsArgPar {})  = False
 
+isHsTypeArg :: HsArg tm ty -> Bool
+isHsTypeArg (HsTypeArg {}) = True
+isHsTypeArg _              = False
+
 isArgPar :: HsArg tm ty -> Bool
 isArgPar (HsArgPar {})  = True
 isArgPar (HsValArg {})  = False
@@ -1283,6 +1287,14 @@ tcArgs :: LHsExpr GhcRn   -- ^ The function itself (for err msgs only)
        -> TcM (HsWrapper, [LHsExprArgOut], TcSigmaType)
           -- ^ (a wrapper for the function, the tc'd args, result type)
 tcArgs fun orig_fun_ty fun_orig orig_args herald
+  | fun_is_out_of_scope
+  , any isHsTypeArg orig_args
+  = failM  -- See Note [VTA for out-of-scope functions]
+    -- We have /already/ emitted a CHoleCan constraint (in tcInferFun),
+    -- which will later cough up a "Variable not in scope error", so
+    -- we can simply fail now, avoiding a confusing error cascade
+
+  | otherwise
   = go [] 1 orig_fun_ty orig_args
   where
     -- Don't count visible type arguments when determining how many arguments
@@ -1291,6 +1303,11 @@ tcArgs fun orig_fun_ty fun_orig orig_args herald
     -- See Note [Herald for matchExpectedFunTys] in TcUnify.
     orig_expr_args_arity = count isHsValArg orig_args
 
+    fun_is_out_of_scope  -- See Note [VTA for out-of-scope functions]
+      = case fun of
+          L _ (HsUnboundVar {}) -> True
+          _                     -> False
+
     go _ _ fun_ty [] = return (idHsWrapper, [], fun_ty)
 
     go acc_args n fun_ty (HsArgPar sp : args)
@@ -1374,6 +1391,33 @@ GHCs we had an ASSERT that Required could not occur here.
 The ice is thin; c.f. Note [No Required TyCoBinder in terms]
 in TyCoRep.
 
+Note [VTA for out-of-scope functions]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Suppose 'wurble' is not in scope, and we have
+   (wurble @Int @Bool True 'x')
+
+Then the renamer will make (HsUnboundVar "wurble) for 'wurble',
+and the typechecker will typecheck it with tcUnboundId, giving it
+a type 'alpha', and emitting a deferred CHoleCan constraint, to
+be reported later.
+
+But then comes the visible type application. If we do nothing, we'll
+generate an immediate failure (in tc_app_err), saying that a function
+of type 'alpha' can't be applied to Bool.  That's insane!  And indeed
+users complain bitterly (#13834, #17150.)
+
+The right error is the CHoleCan, which reports 'wurble' as out of
+scope, and tries to give its type.
+
+Fortunately in tcArgs we still have acces to the function, so
+we can check if it is a HsUnboundVar.  If so, we simply fail
+immediately.  We've already inferred the type of the function,
+so we'll /already/ have emitted a CHoleCan constraint; failing
+preserves that constraint.
+
+A mild shortcoming of this approach is that we thereby
+don't typecheck any of the arguments, but so be it.
+
 Note [Visible type application zonk]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Substitutions should be kind-preserving, so we need kind(tv) = kind(ty_arg).
@@ -1845,8 +1889,8 @@ tcUnboundId rn_expr unbound res_ty
       ; let ev = mkLocalId name ty
       ; can <- newHoleCt (ExprHole unbound) ev ty
       ; emitInsoluble can
-      ; tcWrapResultO (UnboundOccurrenceOf occ) rn_expr (HsVar noExtField (noLoc ev))
-                                                                          ty res_ty }
+      ; tcWrapResultO (UnboundOccurrenceOf occ) rn_expr
+          (HsVar noExtField (noLoc ev)) ty res_ty }
 
 
 {-
diff --git a/testsuite/tests/ghci/scripts/T13466.stderr b/testsuite/tests/ghci/scripts/T13466.stderr
index edd05c5a4d..a31acf385b 100644
--- a/testsuite/tests/ghci/scripts/T13466.stderr
+++ b/testsuite/tests/ghci/scripts/T13466.stderr
@@ -1,7 +1,2 @@
 
 <interactive>:1:1: error: Variable not in scope: out_of_scope
-
-<interactive>:1:1: error:
-    • Cannot apply expression of type ‘t1’
-      to a visible type argument ‘[]’
-    • In the expression: out_of_scope @[]
diff --git a/testsuite/tests/typecheck/should_fail/T12529.stderr b/testsuite/tests/typecheck/should_fail/T12529.stderr
index cd9897e89f..6819cd8dd2 100644
--- a/testsuite/tests/typecheck/should_fail/T12529.stderr
+++ b/testsuite/tests/typecheck/should_fail/T12529.stderr
@@ -1,8 +1,2 @@
 
 T12529.hs:5:5: error: Variable not in scope: p
-
-T12529.hs:5:5: error:
-    • Cannot apply expression of type ‘t1’
-      to a visible type argument ‘Int’
-    • In the expression: p @Int
-      In an equation for ‘f’: f = p @Int
diff --git a/testsuite/tests/typecheck/should_fail/T13834.hs b/testsuite/tests/typecheck/should_fail/T13834.hs
new file mode 100644
index 0000000000..005ea79d68
--- /dev/null
+++ b/testsuite/tests/typecheck/should_fail/T13834.hs
@@ -0,0 +1,6 @@
+{-# LANGUAGE TypeApplications #-}
+
+module Bug where
+
+foo = notInScope @Bool True
+
diff --git a/testsuite/tests/typecheck/should_fail/T13834.stderr b/testsuite/tests/typecheck/should_fail/T13834.stderr
new file mode 100644
index 0000000000..73d739ea87
--- /dev/null
+++ b/testsuite/tests/typecheck/should_fail/T13834.stderr
@@ -0,0 +1,2 @@
+
+T13834.hs:5:7: error: Variable not in scope: notInScope
diff --git a/testsuite/tests/typecheck/should_fail/all.T b/testsuite/tests/typecheck/should_fail/all.T
index fc49dbbb68..a239085b86 100644
--- a/testsuite/tests/typecheck/should_fail/all.T
+++ b/testsuite/tests/typecheck/should_fail/all.T
@@ -541,3 +541,4 @@ test('UnliftedNewtypesConstraintFamily', normal, compile_fail, [''])
 test('UnliftedNewtypesMismatchedKind', normal, compile_fail, [''])
 test('UnliftedNewtypesMismatchedKindRecord', normal, compile_fail, [''])
 test('UnliftedNewtypesMultiFieldGadt', normal, compile_fail, [''])
+test('T13834', normal, compile_fail, [''])