Skip to content

Commit

Permalink
Use a MaxBuffer for Suggestions (#7060)
Browse files Browse the repository at this point in the history 
* Use MaxBuffer for Suggestions

* Extract SuggestNames function

* Rename Parameter

* Apply feedback for formatting
  • Loading branch information
@forki @TIHan
forki authored and TIHan committed Jun 28, 2019
1 parent 010bd00 commit 5a8f454
Show file tree
Hide file tree
Showing 21 changed files with 402 additions and 387 deletions.
42 changes: 24 additions & 18 deletions src/fsharp/CompileOps.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,20 @@ let getErrorString key = SR.GetString key

let (|InvalidArgument|_|) (exn: exn) = match exn with :? ArgumentException as e -> Some e.Message | _ -> None

let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames: bool) =
let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNames: bool) =

let suggestNames suggestionsF idText =
if canSuggestNames then
let buffer = ErrorResolutionHints.SuggestionBuffer idText
if not buffer.Disabled then
suggestionsF buffer.Add
if not buffer.IsEmpty then
os.Append " " |> ignore
os.Append(FSComp.SR.undefinedNameSuggestionsIntro()) |> ignore
for value in buffer do
os.AppendLine() |> ignore
os.Append " " |> ignore
os.Append(DecompileOpName value) |> ignore

let rec OutputExceptionR (os: StringBuilder) error =

Expand Down Expand Up @@ -822,14 +835,10 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames

| UndefinedName(_, k, id, suggestionsF) ->
os.Append(k (DecompileOpName id.idText)) |> ignore
if suggestNames then
let filtered = ErrorResolutionHints.FilterPredictions suggestionsF id.idText
if List.isEmpty filtered |> not then
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore

suggestNames suggestionsF id.idText

| InternalUndefinedItemRef(f, smr, ccuName, s) ->
let _, errs = f(smr, ccuName, s)
let _, errs = f(smr, ccuName, s)
os.Append errs |> ignore

| FieldNotMutable _ ->
Expand Down Expand Up @@ -1363,10 +1372,7 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames

| ErrorWithSuggestions ((_, s), _, idText, suggestionF) ->
os.Append(DecompileOpName s) |> ignore
if suggestNames then
let filtered = ErrorResolutionHints.FilterPredictions suggestionF idText
if List.isEmpty filtered |> not then
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore
suggestNames suggestionF idText

| NumberedError ((_, s), _) -> os.Append s |> ignore

Expand Down Expand Up @@ -1582,10 +1588,10 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames


// remove any newlines and tabs
let OutputPhasedDiagnostic (os: System.Text.StringBuilder) (err: PhasedDiagnostic) (flattenErrors: bool) (suggestNames: bool) =
let OutputPhasedDiagnostic (os: System.Text.StringBuilder) (err: PhasedDiagnostic) (flattenErrors: bool) (canSuggestNames: bool) =
let buf = new System.Text.StringBuilder()

OutputPhasedErrorR buf err suggestNames
OutputPhasedErrorR buf err canSuggestNames
let s = if flattenErrors then ErrorLogger.NormalizeErrorString (buf.ToString()) else buf.ToString()

os.Append s |> ignore
Expand Down Expand Up @@ -1635,7 +1641,7 @@ type Diagnostic =
| Long of bool * DiagnosticDetailedInfo

/// returns sequence that contains Diagnostic for the given error + Diagnostic for all related errors
let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err: PhasedDiagnostic, suggestNames: bool) =
let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err: PhasedDiagnostic, canSuggestNames: bool) =
let outputWhere (showFullPaths, errorStyle) m: DiagnosticLocation =
if Range.equals m rangeStartup || Range.equals m rangeCmdArgs then
{ Range = m; TextRepresentation = ""; IsEmpty = true; File = "" }
Expand Down Expand Up @@ -1708,7 +1714,7 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
let canonical = OutputCanonicalInformation(err.Subcategory(), GetDiagnosticNumber mainError)
let message =
let os = System.Text.StringBuilder()
OutputPhasedDiagnostic os mainError flattenErrors suggestNames
OutputPhasedDiagnostic os mainError flattenErrors canSuggestNames
os.ToString()

let entry: DiagnosticDetailedInfo = { Location = where; Canonical = canonical; Message = message }
Expand All @@ -1723,15 +1729,15 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
let relCanonical = OutputCanonicalInformation(err.Subcategory(), GetDiagnosticNumber mainError) // Use main error for code
let relMessage =
let os = System.Text.StringBuilder()
OutputPhasedDiagnostic os err flattenErrors suggestNames
OutputPhasedDiagnostic os err flattenErrors canSuggestNames
os.ToString()

let entry: DiagnosticDetailedInfo = { Location = relWhere; Canonical = relCanonical; Message = relMessage}
errors.Add( Diagnostic.Long (isError, entry) )

| _ ->
let os = System.Text.StringBuilder()
OutputPhasedDiagnostic os err flattenErrors suggestNames
OutputPhasedDiagnostic os err flattenErrors canSuggestNames
errors.Add( Diagnostic.Short(isError, os.ToString()) )

relatedErrors |> List.iter OutputRelatedError
Expand All @@ -1752,7 +1758,7 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
/// prints error and related errors to the specified StringBuilder
let rec OutputDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError) os (err: PhasedDiagnostic) =

// 'true' for "suggestNames" is passed last here because we want to report suggestions in fsc.exe and fsi.exe, just not in regular IDE usage.
// 'true' for "canSuggestNames" is passed last here because we want to report suggestions in fsc.exe and fsi.exe, just not in regular IDE usage.
let errors = CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err, true)
for e in errors do
Printf.bprintf os "\n"
Expand Down
9 changes: 4 additions & 5 deletions src/fsharp/ConstraintSolver.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2193,12 +2193,11 @@ and ReportNoCandidatesError (csenv: ConstraintSolverEnv) (nUnnamedCallerArgs, nN
match cmeth.UnassignedNamedArgs with
| CallerNamedArg(id, _) :: _ ->
if minfo.IsConstructor then
let predictFields() =
minfo.DeclaringTyconRef.AllInstanceFieldsAsList
|> List.map (fun p -> p.Name.Replace("@", ""))
|> System.Collections.Generic.HashSet
let suggestFields (addToBuffer: string -> unit) =
for p in minfo.DeclaringTyconRef.AllInstanceFieldsAsList do
addToBuffer(p.Name.Replace("@", ""))

ErrorWithSuggestions((msgNum, FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange, id.idText, predictFields)
ErrorWithSuggestions((msgNum, FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange, id.idText, suggestFields)
else
Error((msgNum, FSComp.SR.csMemberHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange)
| [] -> Error((msgNum, msgText), m)
Expand Down
4 changes: 2 additions & 2 deletions src/fsharp/ErrorLogger.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ let rec findOriginalException err =
| WrappedError(err, _) -> findOriginalException err
| _ -> err

type Suggestions = unit -> Collections.Generic.HashSet<string>
type Suggestions = (string -> unit) -> unit

let NoSuggestions : Suggestions = fun () -> Collections.Generic.HashSet()
let NoSuggestions : Suggestions = ignore

/// Thrown when we stop processing the F# Interactive entry or #load.
exception StopProcessingExn of exn option with
Expand Down
133 changes: 76 additions & 57 deletions src/fsharp/ErrorResolutionHints.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ module internal FSharp.Compiler.ErrorResolutionHints

open Internal.Utilities
open FSharp.Compiler.AbstractIL.Internal.Library
open System.Collections
open System.Collections.Generic

let maxSuggestions = 5
let minThresholdForSuggestions = 0.7
let highConfidenceThreshold = 0.85
let minStringLengthForThreshold = 3
let minStringLengthForSuggestion = 3

/// We report a candidate if its edit distance is <= the threshold.
/// The threshold is set to about a quarter of the number of characters.
Expand All @@ -23,65 +25,82 @@ let IsInEditDistanceProximity idText suggestion =

editDistance <= threshold

/// Filters predictions based on edit distance to the given unknown identifier.
let FilterPredictions (suggestionF:ErrorLogger.Suggestions) (idText:string) =
/// Demangles a suggestion
let DemangleOperator (nm: string) =
if nm.StartsWithOrdinal("( ") && nm.EndsWithOrdinal(" )") then
nm.[2..nm.Length - 3]
else
nm

type SuggestionBufferEnumerator(tail: int, data: KeyValuePair<float,string> []) =
let mutable current = data.Length
interface IEnumerator<string> with
member __.Current
with get () =
let kvpr = &data.[current]
kvpr.Value
interface System.Collections.IEnumerator with
member __.Current with get () = box data.[current].Value
member __.MoveNext() =
current <- current - 1
current > tail || (current = tail && data.[current] <> Unchecked.defaultof<_>)
member __.Reset () = current <- data.Length
interface System.IDisposable with
member __.Dispose () = ()

type SuggestionBuffer(idText: string) =
let data = Array.zeroCreate<KeyValuePair<float,string>>(maxSuggestions)
let mutable tail = maxSuggestions - 1
let uppercaseText = idText.ToUpperInvariant()
let allSuggestions = suggestionF()
let dotIdText = "." + idText
let mutable disableSuggestions = idText.Length < minStringLengthForSuggestion

let demangle (nm:string) =
if nm.StartsWithOrdinal("( ") && nm.EndsWithOrdinal(" )") then
let cleanName = nm.[2..nm.Length - 3]
cleanName
else nm
let insert (k,v) =
let mutable pos = tail
while pos < maxSuggestions && (let kv = &data.[pos] in kv.Key < k) do
pos <- pos + 1

/// Returns `true` if given string is an operator display name, e.g. ( |>> )
let IsOperatorName (name: string) =
if isNull name || not (name.StartsWithOrdinal("( ") && name.EndsWithOrdinal(" )")) then
false
else
let mutable i = 2
let mutable isOperator = true
while isOperator && i < name.Length - 3 do
if name.[i] = ' ' then
isOperator <- false
i <- i + 1
isOperator
if pos > 0 then
if pos >= maxSuggestions || (let kv = &data.[pos] in k <> kv.Key || v <> kv.Value) then
if tail < pos - 1 then
for i = tail to pos - 2 do
data.[i] <- data.[i + 1]
data.[pos - 1] <- KeyValuePair(k,v)
if tail > 0 then tail <- tail - 1

if allSuggestions.Contains idText then [] else // some other parsing error occurred
let dotIdText = "." + idText
allSuggestions
|> Seq.choose (fun suggestion ->
// Because beginning a name with _ is used both to indicate an unused
// value as well as to formally squelch the associated compiler
// error/warning (FS1182), we remove such names from the suggestions,
// both to prevent accidental usages as well as to encourage good taste
if IsOperatorName suggestion || suggestion.StartsWithOrdinal("_") then None else
let suggestion:string = demangle suggestion
let suggestedText = suggestion.ToUpperInvariant()
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
if similarity >= highConfidenceThreshold || suggestion.EndsWithOrdinal(dotIdText) then
Some(similarity, suggestion)
elif similarity < minThresholdForSuggestions && suggestedText.Length > minStringLengthForThreshold then
None
elif IsInEditDistanceProximity uppercaseText suggestedText then
Some(similarity, suggestion)
else
None)
|> Seq.sortByDescending fst
|> Seq.mapi (fun i x -> i, x)
|> Seq.takeWhile (fun (i, _) -> i < maxSuggestions)
|> Seq.map snd
|> Seq.toList
member __.Add (suggestion: string) =
if not disableSuggestions then
if suggestion = idText then // some other parse error happened
disableSuggestions <- true

// Because beginning a name with _ is used both to indicate an unused
// value as well as to formally squelch the associated compiler
// error/warning (FS1182), we remove such names from the suggestions,
// both to prevent accidental usages as well as to encourage good taste
if suggestion.Length >= minStringLengthForSuggestion && not (suggestion.StartsWithOrdinal "_") then
let suggestion:string = DemangleOperator suggestion
let suggestedText = suggestion.ToUpperInvariant()
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
if similarity >= highConfidenceThreshold ||
suggestion.EndsWithOrdinal dotIdText ||
(similarity >= minThresholdForSuggestions && IsInEditDistanceProximity uppercaseText suggestedText)
then
insert(similarity, suggestion) |> ignore

member __.Disabled with get () = disableSuggestions

member __.IsEmpty with get () = disableSuggestions || (tail = maxSuggestions - 1)

/// Formats the given predictions according to the error style.
let FormatPredictions normalizeF (predictions: (float * string) list) =
match predictions with
| [] -> System.String.Empty
| _ ->
let suggestions =
predictions
|> List.map (snd >> normalizeF)
|> List.map (sprintf "%s %s" System.Environment.NewLine)
|> String.concat ""
interface IEnumerable<string> with
member this.GetEnumerator () =
if this.IsEmpty then
Seq.empty.GetEnumerator()
else
new SuggestionBufferEnumerator(tail, data) :> IEnumerator<string>

" " + FSComp.SR.undefinedNameSuggestionsIntro() + suggestions
interface IEnumerable with
member this.GetEnumerator () =
if this.IsEmpty then
Seq.empty.GetEnumerator() :> IEnumerator
else
new SuggestionBufferEnumerator(tail, data) :> IEnumerator
Loading

0 comments on commit 5a8f454

Please sign in to comment.