Skip to content

Commit

Permalink
Merge pull request #906 from Thorium/code-cleanup
Browse files Browse the repository at this point in the history
Minor code clean-ups and performance improvements
  • Loading branch information
nojaf authored Mar 15, 2024
2 parents b50b70e + c4f4e0d commit 1c4b2e0
Show file tree
Hide file tree
Showing 19 changed files with 84 additions and 69 deletions.
4 changes: 2 additions & 2 deletions src/Common/Collections.fs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ module internal List =
f x
g ()
iterInterleaved f g (y :: tl)
| x :: [] -> f x
| [ x ] -> f x
| [] -> ()

/// Tests whether a list starts with the elements of another
Expand Down Expand Up @@ -90,7 +90,7 @@ module internal List =
let other, rest = partitionUntil (f >> not) other
yield last, other
yield! loop rest
| [] when other = [] -> ()
| [] when List.isEmpty other -> ()
| _ -> invalidArg "" "Should start with true"
}

Expand Down
40 changes: 25 additions & 15 deletions src/Common/StringParsing.fs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ open FSharp.Formatting.Markdown

module String =
/// Matches when a string is a whitespace or null
[<return: Struct>]
let (|WhiteSpace|_|) (s) =
if String.IsNullOrWhiteSpace(s) then Some() else None
if String.IsNullOrWhiteSpace(s) then
ValueSome()
else
ValueNone

/// Returns a string trimmed from both start and end
let (|TrimBoth|) (text: string) = text.Trim()
Expand Down Expand Up @@ -117,15 +121,20 @@ module String =

module StringPosition =
/// Matches when a string is a whitespace or null
[<return: Struct>]
let (|WhiteSpace|_|) (s, _n: MarkdownRange) =
if String.IsNullOrWhiteSpace(s) then Some() else None
if String.IsNullOrWhiteSpace(s) then
ValueSome()
else
ValueNone

/// Matches when a string does starts with non-whitespace
[<return: Struct>]
let (|Unindented|_|) (s: string, _n: MarkdownRange) =
if not (String.IsNullOrWhiteSpace(s)) && s.TrimStart() = s then
Some()
ValueSome()
else
None
ValueNone

/// Returns a string trimmed from both start and end
let (|TrimBoth|) (text: string, n: MarkdownRange) =
Expand Down Expand Up @@ -174,11 +183,12 @@ module StringPosition =
StartColumn = n.StartColumn + text.Length - trimmed.Length })

/// Matches when a string starts with any of the specified sub-strings
[<return: Struct>]
let (|StartsWithAny|_|) (starts: string seq) (text: string, _n: MarkdownRange) =
if starts |> Seq.exists (fun s -> text.StartsWith(s, StringComparison.Ordinal)) then
Some()
ValueSome()
else
None
ValueNone

/// Matches when a string starts with the specified sub-string
let (|StartsWith|_|) (start: string) (text: string, n: MarkdownRange) =
Expand Down Expand Up @@ -297,15 +307,16 @@ module StringPosition =

/// Matches when a string consists of some number of
/// complete repetitions of a specified sub-string.
[<return: Struct>]
let (|EqualsRepeated|_|) (repeated, _n: MarkdownRange) =
function
| StartsWithRepeated repeated (_n, (v, _)) when (String.IsNullOrWhiteSpace v) -> Some()
| _ -> None
| StartsWithRepeated repeated (_n, (v, _)) when (String.IsNullOrWhiteSpace v) -> ValueSome()
| _ -> ValueNone

module List =
/// Matches a list if it starts with a sub-list that is delimited
/// using the specified delimiters. Returns a wrapped list and the rest.
let inline (|DelimitedWith|_|) startl endl input =
let inline internal (|DelimitedWith|_|) startl endl input =
if List.startsWith startl input then
match List.partitionUntilEquals endl (List.skip startl.Length input) with
| Some(pre, post) -> Some(pre, List.skip endl.Length post, startl.Length, endl.Length)
Expand All @@ -314,14 +325,14 @@ module List =
None

/// Matches a list if it starts with a sub-list. Returns the list.
let inline (|StartsWith|_|) startl input =
let inline internal (|StartsWith|_|) startl input =
if List.startsWith startl input then Some input else None

/// Matches a list if it starts with a sub-list that is delimited
/// using the specified delimiter. Returns a wrapped list and the rest.
let inline (|Delimited|_|) str = (|DelimitedWith|_|) str str
let inline internal (|Delimited|_|) str = (|DelimitedWith|_|) str str

let inline (|DelimitedNTimes|_|) str input =
let inline internal (|DelimitedNTimes|_|) str input =
let strs, _items = List.partitionWhile (fun i -> i = str) input

match strs with
Expand Down Expand Up @@ -403,9 +414,8 @@ module Lines =
let (|TrimParagraphLines|) lines =
lines
// first remove all whitespace on the beginning of the line
|> List.map (fun (StringPosition.TrimStart s) -> s)
// Now remove all additional spaces at the end, but keep two spaces if existent
|> List.map (fun (s, n) ->
// then remove all additional spaces at the end, but keep two spaces if existent
|> List.map (fun (StringPosition.TrimStart(s, n)) ->
let endsWithTwoSpaces = s.EndsWith(" ", StringComparison.Ordinal)

let trimmed = s.TrimEnd([| ' ' |]) + if endsWithTwoSpaces then " " else ""
Expand Down
4 changes: 2 additions & 2 deletions src/FSharp.Formatting.ApiDocs/Categorise.fs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ let entities (nsIndex: int, ns: ApiDocNamespace, suppress) =
//
// See https://github.com/fsharp/fsharp-core-docs/issues/57, we may rethink this
|> List.filter (fun e ->
not (e.Symbol.Namespace = Some "Microsoft.FSharp.Data.UnitSystems.SI.UnitSymbols"))
(e.Symbol.Namespace <> Some "Microsoft.FSharp.Data.UnitSystems.SI.UnitSymbols"))
|> List.filter (fun e ->
not (e.Symbol.Namespace = Some "Microsoft.FSharp.Data.UnitSystems.SI.UnitNames"))
(e.Symbol.Namespace <> Some "Microsoft.FSharp.Data.UnitSystems.SI.UnitNames"))
// Don't show 'AnonymousObject' in list-of-namespaces navigation
|> List.filter (fun e ->
not (
Expand Down
21 changes: 11 additions & 10 deletions src/FSharp.Formatting.ApiDocs/GenerateModel.fs
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,16 @@ module internal Utils =
else
None

[<return: Struct>]
let (|MeasureOne|_|) (typ: FSharpType) =
if
typ.HasTypeDefinition
&& typ.TypeDefinition.LogicalName = "1"
&& typ.GenericArguments.Count = 0
then
Some()
ValueSome()
else
None
ValueNone

let tryGetLocation (symbol: FSharpSymbol) =
match symbol.ImplementationLocation with
Expand Down Expand Up @@ -105,7 +106,7 @@ module internal Utils =
member x.TryAttr(attr: string) =
let a = x.Attribute(XName.Get attr)

if a = null then None
if isNull a then None
else if String.IsNullOrEmpty a.Value then None
else Some a.Value

Expand Down Expand Up @@ -1757,7 +1758,7 @@ module internal SymbolReader =
[ let mutable line = ""

while (line <- reader.ReadLine()
line <> null) do
not (isNull line)) do
yield line ]

String.removeSpaces lines
Expand Down Expand Up @@ -1905,14 +1906,14 @@ module internal SymbolReader =
let name = elem.Attribute(XName.Get "name")
let nameAsHtml = HttpUtility.HtmlEncode name.Value

if name <> null then
if not (isNull name) then
html.AppendFormat("<span class=\"fsdocs-param-name\">{0}</span>", nameAsHtml)
|> ignore
| "see"
| "seealso" ->
let cref = elem.Attribute(XName.Get "cref")

if cref <> null then
if not (isNull cref) then
if System.String.IsNullOrEmpty(cref.Value) || cref.Value.Length < 3 then
printfn "ignoring invalid cref specified in: %A" e

Expand Down Expand Up @@ -2018,7 +2019,7 @@ module internal SymbolReader =
let remarks =
let remarkNodes = doc.Elements(XName.Get "remarks") |> Seq.toList

if List.length remarkNodes > 0 then
if not (List.isEmpty remarkNodes) then
let html = new StringBuilder()

for (id, e) in List.indexed remarkNodes do
Expand Down Expand Up @@ -2053,7 +2054,7 @@ module internal SymbolReader =
[ for e in exceptionNodes do
let cref = e.Attribute(XName.Get "cref")

if cref <> null then
if not (isNull cref) then
if String.IsNullOrEmpty(cref.Value) || cref.Value.Length < 3 then
printfn "Warning: Invalid cref specified in: %A" doc

Expand Down Expand Up @@ -2581,7 +2582,7 @@ module internal SymbolReader =
|> Seq.choose (fun p ->
let nameAttr = p.Attribute(XName.Get "name")

if nameAttr = null then
if isNull nameAttr then
None
else
Some(nameAttr.Value, p.Value))
Expand Down Expand Up @@ -2821,7 +2822,7 @@ module internal SymbolReader =
[ for e in doc.Descendants(XName.Get "member") do
let attr = e.Attribute(XName.Get "name")

if attr <> null && not (String.IsNullOrEmpty(attr.Value)) then
if (not (isNull attr)) && not (String.IsNullOrEmpty(attr.Value)) then
yield attr.Value, e ] do
// NOTE: We completely ignore duplicate keys and I don't see
// an easy way to detect where "value" is coming from, because the entries
Expand Down
2 changes: 1 addition & 1 deletion src/FSharp.Formatting.CodeFormat/CodeFormatAgent.fs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ module CodeFormatter =
[| let line = ref ""

while (line := reader.ReadLine()
line.Value <> null) do
not (isNull line.Value)) do
yield line.Value |]
// Get options for a standalone script file (this adds some
// default references and doesn't require full project information)
Expand Down
4 changes: 2 additions & 2 deletions src/FSharp.Formatting.CodeFormat/SourceCode.fs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type ToolTipSpan =
| HardLineBreak

/// Classifies tokens reported by the FCS
[<RequireQualifiedAccess>]
[<RequireQualifiedAccess; Struct>]
type TokenKind =
| Keyword
| String
Expand Down Expand Up @@ -49,7 +49,7 @@ type TokenKind =


/// Represents a kind of error reported from the F# compiler (warning or error)
[<RequireQualifiedAccess>]
[<RequireQualifiedAccess; Struct>]
type ErrorKind =
| Error
| Warning
Expand Down
3 changes: 1 addition & 2 deletions src/FSharp.Formatting.Common/PynbModel.fs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ type Cell =
| Some(x) -> string<int> x)
(this.outputs |> Array.map string<Output> |> String.concat ",\n")))
(this.source
|> Array.map addLineEnd
|> Array.map escapeAndQuote
|> Array.map (addLineEnd >> escapeAndQuote)
|> String.concat ",\n ")

type Kernelspec =
Expand Down
14 changes: 6 additions & 8 deletions src/FSharp.Formatting.Common/Templating.fs
Original file line number Diff line number Diff line change
Expand Up @@ -280,14 +280,12 @@ module internal SimpleTemplating =
// If there is no template or the template is an empty file, return just document + tooltips (tooltips empty if not HTML)
let lookup = readOnlyDict substitutions

(if lookup.ContainsKey ParamKeys.``fsdocs-content`` then
lookup.[ParamKeys.``fsdocs-content``]
else
"")
+ (if lookup.ContainsKey ParamKeys.``fsdocs-tooltips`` then
"\n\n" + lookup.[ParamKeys.``fsdocs-tooltips``]
else
"")
(match lookup.TryGetValue ParamKeys.``fsdocs-content`` with
| true, lookupContent -> lookupContent
| false, _ -> "")
+ (match lookup.TryGetValue ParamKeys.``fsdocs-tooltips`` with
| true, lookupTips -> "\n\n" + lookupTips
| false, _ -> "")
| Some templateText -> ApplySubstitutionsInText substitutions templateText

let UseFileAsSimpleTemplate (substitutions, templateOpt, outputFile) =
Expand Down
23 changes: 12 additions & 11 deletions src/FSharp.Formatting.Common/YaafFSharpScripting.fs
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,9 @@ module internal CompilerServiceExtensions =
dllFiles
|> List.map (fun file ->
file,
(if referenceDict.ContainsKey file then
Some referenceDict.[file]
else
None))
(match referenceDict.TryGetValue file with
| true, refFile -> Some refFile
| false, _ -> None))

let getProjectReferencesSimple frameworkVersion (dllFiles: string list) =
getProjectReferences frameworkVersion None None dllFiles |> resolve dllFiles
Expand Down Expand Up @@ -464,26 +463,29 @@ module internal ArgParser =
else
None

[<return: Struct>]
let (|FsiBoolArg|_|) argName s =
match s with
| StartsWith argName rest ->
if String.IsNullOrWhiteSpace rest then
Some true
ValueSome true
else
match rest with
| "+" -> Some true
| "-" -> Some false
| _ -> None
| _ -> None
| "+" -> ValueSome true
| "-" -> ValueSome false
| _ -> ValueNone
| _ -> ValueNone

open ArgParser

[<Struct>]
type internal DebugMode =
| Full
| PdbOnly
| Portable
| NoDebug

[<Struct>]
type internal OptimizationType =
| NoJitOptimize
| NoJitTracking
Expand Down Expand Up @@ -761,7 +763,7 @@ type internal FsiOptions =
| [] -> Seq.empty
| opts ->
opts
|> Seq.map (fun (enable, types) ->
|> Seq.collect (fun (enable, types) ->
seq {
yield sprintf "--optimize%s" (getMinusPlus enable)

Expand All @@ -778,7 +780,6 @@ type internal FsiOptions =
| NoTailCalls -> "notailcalls")
|> String.concat ","
})
|> Seq.concat

yield! getSimpleBoolArg "--quiet" x.Quiet
yield! getSimpleBoolArg "--quotations-debug" x.QuotationsDebug
Expand Down
2 changes: 1 addition & 1 deletion src/FSharp.Formatting.Literate/Contexts.fs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type internal CompilerContext =
}

/// Defines the possible output types from literate script (HTML, Latex, Pynb)
[<RequireQualifiedAccess>]
[<RequireQualifiedAccess; Struct>]
type OutputKind =
/// Requests HTML output
| Html
Expand Down
2 changes: 1 addition & 1 deletion src/FSharp.Formatting.Literate/Evaluator.fs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ open FSharp.Formatting.Internal
/// <namespacedoc>
/// <summary>Functionality to support literate evaluation for F# scripts</summary>
/// </namespacedoc>
[<RequireQualifiedAccessAttribute>]
[<RequireQualifiedAccessAttribute; Struct>]
type FsiEmbedKind =
/// The FSI output
| FsiOutput
Expand Down
6 changes: 4 additions & 2 deletions src/FSharp.Formatting.Literate/ParseScript.fs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ type internal ParseScript(parseOptions, ctx: CompilerContext) =
| Command "condition" name when not (String.IsNullOrWhiteSpace name) -> { Condition = Some name }
| _ -> { Condition = None }

let (|EmptyString|_|) (v: string) = if v.Length = 0 then Some() else None
[<return: Struct>]
let (|EmptyString|_|) (v: string) =
if v.Length = 0 then ValueSome() else ValueNone

/// Transform list of code blocks (snippet/comment/command)
/// into a formatted Markdown document, with link definitions
Expand Down Expand Up @@ -386,7 +388,7 @@ type internal ParseScript(parseOptions, ctx: CompilerContext) =

let parsedBlocks =
[ for Snippet(name, lines) in sourceSnippets do
if name <> null then
if not (isNull name) then
yield BlockComment("## " + name)

yield! parseScriptFile (lines) ]
Expand Down
Loading

0 comments on commit 1c4b2e0

Please sign in to comment.