Suggestions for misspelled values, namespaces, attributes, functions and more - fixes dotnet#1909 (dotnet#1911)

* Suggestions for mispelled values, namespaces and co - fixes dotnet#1909

* Do not discard specialized errors for undefined message - fixes dotnet#1933

* fix tests

* Test for suggestion in generic type

* Flatten predictions in ErrorLogger when in IDE

* Fix ranges

* Fix tests

* Regularize all undefined name messages

* Suggest type parameters

* Suggest attributes

* Suggest Methods

* fix tests

* Optimize perf

* Calculate all predictions lazily

* Be more careful with error allocations

* Remove one a string comparision per Resolver call

* fix merge error

Author: forki

src/fsharp/CompileOps.fs

@@ -156,6 +156,7 @@ let GetRangeOfError(err:PhasedError) =
       | RecursiveUseCheckedAtRuntime (_,_,m) 
       | LetRecEvaluatedOutOfOrder (_,_,_,m) 
       | Error (_,m)
+      | ErrorWithPredictions (_,m,_,_)
       | NumberedError (_,m)
       | SyntaxError (_,m) 
       | InternalError (_,m)
@@ -369,6 +370,7 @@ let GetErrorNumber(err:PhasedError) =
       | WrappedError(e,_) -> GetFromException e   
 
       | Error ((n,_),_) -> n
+      | ErrorWithPredictions ((n,_),_,_,_) -> n
       | Failure _ -> 192
       | NumberedError((n,_),_) -> n
       | IllegalFileNameChar(fileName,invalidChar) -> fst (FSComp.SR.buildUnexpectedFileNameCharacter(fileName,string invalidChar))
@@ -387,6 +389,7 @@ let GetWarningLevel err =
   | DefensiveCopyWarning _
   | FullAbstraction _ ->  5
   | NumberedError((n,_),_) 
+  | ErrorWithPredictions((n,_),_,_,_) 
   | Error((n,_),_) -> 
       // 1178,tcNoComparisonNeeded1,"The struct, record or union type '%s' is not structurally comparable because the type parameter %s does not satisfy the 'comparison' constraint. Consider adding the 'NoComparison' attribute to this type to clarify that the type is not comparable"
       // 1178,tcNoComparisonNeeded2,"The struct, record or union type '%s' is not structurally comparable because the type '%s' does not satisfy the 'comparison' constraint. Consider adding the 'NoComparison' attribute to this type to clarify that the type is not comparable" 
@@ -606,7 +609,7 @@ let getErrorString key = SR.GetString key
 
 let (|InvalidArgument|_|) (exn:exn) = match exn with :? ArgumentException as e -> Some e.Message | _ -> None
 
-let OutputPhasedErrorR (os:System.Text.StringBuilder) (err:PhasedError) =
+let OutputPhasedErrorR errorStyle (os:System.Text.StringBuilder) (err:PhasedError) =
     let rec OutputExceptionR (os:System.Text.StringBuilder) = function        
       | ConstraintSolverTupleDiffLengths(_,tl1,tl2,m,m2) -> 
           os.Append(ConstraintSolverTupleDiffLengthsE().Format tl1.Length tl2.Length) |> ignore
@@ -774,11 +777,11 @@ let OutputPhasedErrorR (os:System.Text.StringBuilder) (err:PhasedError) =
               os.Append(Duplicate1E().Format (DecompileOpName s)) |> ignore
           else 
               os.Append(Duplicate2E().Format k (DecompileOpName s)) |> ignore
-      | UndefinedName(_,k,id,predictions) ->
+      | UndefinedName(_,k,id,predictionsF) ->
           os.Append(k (DecompileOpName id.idText)) |> ignore
-          if Set.isEmpty predictions |> not then
-              let filtered = ErrorResolutionHints.FilterPredictions id.idText predictions
-              os.Append(ErrorResolutionHints.FormatPredictions filtered) |> ignore
+          let filtered = ErrorResolutionHints.FilterPredictions id.idText predictionsF
+          if List.isEmpty filtered |> not then
+              os.Append(ErrorResolutionHints.FormatPredictions errorStyle DecompileOpName filtered) |> ignore
 
       | InternalUndefinedItemRef(f,smr,ccuName,s) ->  
           let _, errs = f(smr, ccuName, s)  
@@ -1258,13 +1261,18 @@ let OutputPhasedErrorR (os:System.Text.StringBuilder) (err:PhasedError) =
           os.Append(NonUniqueInferredAbstractSlot1E().Format bindnm) |> ignore
           let ty1 = bvirt1.EnclosingType
           let ty2 = bvirt2.EnclosingType
-          // REVIEW: consider if we need to show _cxs (the type parameter constrants)
+          // REVIEW: consider if we need to show _cxs (the type parameter constraints)
           let t1, t2, _cxs = NicePrint.minimalStringsOfTwoTypes denv ty1 ty2
           os.Append(NonUniqueInferredAbstractSlot2E().Format) |> ignore
           if t1 <> t2 then 
               os.Append(NonUniqueInferredAbstractSlot3E().Format t1 t2) |> ignore
           os.Append(NonUniqueInferredAbstractSlot4E().Format) |> ignore
       | Error ((_,s),_) -> os.Append(s) |> ignore
+      | ErrorWithPredictions ((_,s),_,idText,predictionsF) -> 
+          os.Append(DecompileOpName s) |> ignore
+          let filtered = ErrorResolutionHints.FilterPredictions idText predictionsF
+          if List.isEmpty filtered |> not then
+              os.Append(ErrorResolutionHints.FormatPredictions errorStyle DecompileOpName filtered) |> ignore
       | NumberedError ((_,s),_) -> os.Append(s) |> ignore
       | InternalError (s,_) 
       | InvalidArgument s 
@@ -1416,22 +1424,14 @@ let OutputPhasedErrorR (os:System.Text.StringBuilder) (err:PhasedError) =
 
 
 // remove any newlines and tabs 
-let OutputPhasedError (os:System.Text.StringBuilder) (err:PhasedError) (flattenErrors:bool) = 
+let OutputPhasedError errorStyle (os:System.Text.StringBuilder) (err:PhasedError) (flattenErrors:bool) = 
     let buf = new System.Text.StringBuilder()
 
-    OutputPhasedErrorR buf err
+    OutputPhasedErrorR errorStyle buf err
     let s = if flattenErrors then ErrorLogger.NormalizeErrorString (buf.ToString()) else buf.ToString()
 
     os.Append(s) |> ignore
 
-
-type ErrorStyle = 
-    | DefaultErrors 
-    | EmacsErrors 
-    | TestErrors 
-    | VSErrors
-    | GccErrors
-
 let SanitizeFileName fileName implicitIncludeDir =
     // The assert below is almost ok, but it fires in two cases:
     //  - fsi.exe sometimes passes "stdin" as a dummy filename
@@ -1550,7 +1550,7 @@ let CollectErrorOrWarning (implicitIncludeDir,showFullPaths,flattenErrors,errorS
             let canonical = OutputCanonicalInformation(err.Subcategory(),GetErrorNumber mainError)
             let message = 
                 let os = System.Text.StringBuilder()
-                OutputPhasedError os mainError flattenErrors
+                OutputPhasedError errorStyle os mainError flattenErrors
                 os.ToString()
 
             let entry : DetailedIssueInfo = { Location = where; Canonical = canonical; Message = message }
@@ -1565,15 +1565,15 @@ let CollectErrorOrWarning (implicitIncludeDir,showFullPaths,flattenErrors,errorS
                     let relCanonical = OutputCanonicalInformation(err.Subcategory(),GetErrorNumber mainError) // Use main error for code
                     let relMessage = 
                         let os = System.Text.StringBuilder()
-                        OutputPhasedError os err flattenErrors
+                        OutputPhasedError errorStyle os err flattenErrors
                         os.ToString()
 
                     let entry : DetailedIssueInfo = { Location = relWhere; Canonical = relCanonical; Message = relMessage}
                     errors.Add( ErrorOrWarning.Long (not warn, entry) )
 
                 | _ -> 
                     let os = System.Text.StringBuilder()
-                    OutputPhasedError os err flattenErrors
+                    OutputPhasedError errorStyle os err flattenErrors
                     errors.Add( ErrorOrWarning.Short((not warn), os.ToString()) )
 
             relatedErrors |> List.iter OutputRelatedError

src/fsharp/CompileOps.fsi

@@ -73,14 +73,6 @@ val ParseInput : (UnicodeLexing.Lexbuf -> Parser.token) * ErrorLogger * UnicodeL
 // Error and warnings
 //--------------------------------------------------------------------------
 
-/// Represents the style being used to format errros
-type ErrorStyle = 
-    | DefaultErrors 
-    | EmacsErrors 
-    | TestErrors 
-    | VSErrors
-    | GccErrors
-
 /// Get the location associated with an error
 val GetRangeOfError : PhasedError -> range option
 
@@ -91,7 +83,7 @@ val GetErrorNumber : PhasedError -> int
 val SplitRelatedErrors : PhasedError -> PhasedError * PhasedError list
 
 /// Output an error to a buffer
-val OutputPhasedError : StringBuilder -> PhasedError -> bool -> unit
+val OutputPhasedError : ErrorLogger.ErrorStyle -> StringBuilder -> PhasedError -> bool -> unit
 
 /// Output an error or warning to a buffer
 val OutputErrorOrWarning : implicitIncludeDir:string * showFullPaths: bool * flattenErrors: bool * errorStyle: ErrorStyle *  warning:bool -> StringBuilder -> PhasedError -> unit

src/fsharp/ConstraintSolver.fs

@@ -2022,22 +2022,18 @@ and ReportNoCandidatesError (csenv:ConstraintSolverEnv) (nUnnamedCallerArgs,nNam
     | _,_,_,_,([],[cmeth]) -> 
         let minfo = cmeth.Method
         let msgNum,msgText = FSComp.SR.csRequiredSignatureIs(NicePrint.stringOfMethInfo amap m denv minfo)
-        let msgNum,msgText,msgRange = 
-            match cmeth.UnassignedNamedArgs with 
-            | CallerNamedArg(id,_) :: _ -> 
-                if minfo.IsConstructor then
-                    let typ = minfo.DeclaringEntityRef
-
-                    let predictions =
-                        typ.AllInstanceFieldsAsList
-                        |> List.map (fun p -> p.Name.Replace("@",""))
-                        |> ErrorResolutionHints.FilterPredictions id.idText
-
-                    msgNum,FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText, ErrorResolutionHints.FormatPredictions predictions),id.idRange
-                else
-                    msgNum,FSComp.SR.csMemberHasNoArgumentOrReturnProperty(methodName, id.idText, msgText),id.idRange
-            | [] -> (msgNum,msgText,m)
-        Error ((msgNum,msgText),msgRange)
+        match cmeth.UnassignedNamedArgs with 
+        | CallerNamedArg(id,_) :: _ -> 
+            if minfo.IsConstructor then
+                let predictFields() =
+                    minfo.DeclaringEntityRef.AllInstanceFieldsAsList
+                    |> List.map (fun p -> p.Name.Replace("@",""))
+                    |> Set.ofList
+
+                ErrorWithPredictions((msgNum,FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)),id.idRange,id.idText,predictFields)
+            else
+                Error((msgNum,FSComp.SR.csMemberHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)),id.idRange)
+        | [] -> Error((msgNum,msgText),m)
 
     // One method, incorrect number of arguments provided by the user
     | _,_,([],[cmeth]),_,_ when not cmeth.HasCorrectArity ->  

src/fsharp/ErrorLogger.fs

@@ -14,6 +14,15 @@ open System
 // General error recovery mechanism
 //-----------------------------------------------------------------------
 
+/// Represents the style being used to format errros
+[<RequireQualifiedAccess>]
+type ErrorStyle = 
+| DefaultErrors 
+| EmacsErrors 
+| TestErrors 
+| VSErrors
+| GccErrors
+
 /// Thrown when we want to add some range information to a .NET exception
 exception WrappedError of exn * range
 
@@ -37,6 +46,10 @@ let rec findOriginalException err =
     | _ -> err
 
 
+type Predictions = unit -> Set<string>
+
+let NoPredictions : Predictions = fun () -> Set.empty
+
 /// Thrown when we stop processing the F# Interactive entry or #load.
 exception StopProcessingExn of exn option
 let (|StopProcessing|_|) exn = match exn with StopProcessingExn _ -> Some () | _ -> None
@@ -48,11 +61,20 @@ exception NumberedError of (int * string) * range with   // int is e.g. 191 in F
         match this :> exn with
         | NumberedError((_,msg),_) -> msg
         | _ -> "impossible"
+
 exception Error of (int * string) * range with   // int is e.g. 191 in FS0191  // eventually remove this type, it is a transitional artifact of the old unnumbered error style
     override this.Message =
         match this :> exn with
         | Error((_,msg),_) -> msg
         | _ -> "impossible"
+
+
+exception ErrorWithPredictions of (int * string) * range * string * Predictions with   // int is e.g. 191 in FS0191 
+    override this.Message =
+        match this :> exn with
+        | ErrorWithPredictions((_,msg),_,_,_) -> msg
+        | _ -> "impossible"
+
 exception InternalError of string * range
 exception UserCompilerMessage of string * int * range
 exception LibraryUseOnly of range
@@ -517,7 +539,7 @@ let NewlineifyErrorString (message:string) = message.Replace(stringThatIsAProxyF
 /// fixes given string by replacing all control chars with spaces.
 /// NOTE: newlines are recognized and replaced with stringThatIsAProxyForANewlineInFlatErrors (ASCII 29, the 'group separator'), 
 /// which is decoded by the IDE with 'NewlineifyErrorString' back into newlines, so that multi-line errors can be displayed in QuickInfo
-let NormalizeErrorString (text : string) =    
+let NormalizeErrorString (text : string) =
     if isNull text then nullArg "text"
     let text = text.Trim()
 
@@ -530,7 +552,7 @@ let NormalizeErrorString (text : string) =
                 // handle \r\n sequence - replace it with one single space
                 buf.Append(stringThatIsAProxyForANewlineInFlatErrors) |> ignore
                 2
-            | '\n' ->
+            | '\n' | '\r' ->
                 buf.Append(stringThatIsAProxyForANewlineInFlatErrors) |> ignore
                 1
             | c ->

src/fsharp/ErrorResolutionHints.fs

@@ -4,26 +4,36 @@
 module internal Microsoft.FSharp.Compiler.ErrorResolutionHints
 
 /// Filters predictions based on edit distance to an unknown identifier.
-let FilterPredictions unknownIdent allPredictions =
+let FilterPredictions (unknownIdent:string) (predictionsF:ErrorLogger.Predictions) =
     let rec take n predictions = 
         predictions 
         |> Seq.mapi (fun i x -> i,x) 
         |> Seq.takeWhile (fun (i,_) -> i < n) 
         |> Seq.map snd 
         |> Seq.toList
 
-    allPredictions
-    |> Seq.toList
-    |> List.distinct
-    |> List.sortByDescending (Internal.Utilities.EditDistance.JaroWinklerDistance unknownIdent)
+    let unknownIdent = unknownIdent.ToUpperInvariant()
+    predictionsF()
+    |> Seq.sortByDescending (fun p -> Internal.Utilities.EditDistance.JaroWinklerDistance unknownIdent (p.ToUpperInvariant()))
     |> take 5
 
-let FormatPredictions predictions =
+let FormatPredictions errorStyle normalizeF predictions =
     match predictions with
     | [] -> System.String.Empty
     | _ ->
-        let predictionText =
-            predictions 
-            |> Seq.map (sprintf "%s   %s" System.Environment.NewLine) 
-            |> String.concat ""
-        System.Environment.NewLine + FSComp.SR.undefinedNameRecordLabelDetails() + predictionText
+        match errorStyle with
+        | ErrorLogger.ErrorStyle.VSErrors ->
+            let predictionText =
+                predictions 
+                |> List.map normalizeF
+                |> String.concat ", "
+
+            " " + FSComp.SR.undefinedNameRecordLabelDetails() + " " + predictionText
+        | _ ->
+            let predictionText =
+                predictions 
+                |> List.map normalizeF
+                |> Seq.map (sprintf "%s   %s" System.Environment.NewLine) 
+                |> String.concat ""
+
+            System.Environment.NewLine + FSComp.SR.undefinedNameRecordLabelDetails() + predictionText