Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ordinal string comparison in string manipulation methods #4912

Merged
merged 5 commits into from
Jun 12, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions fcs/FSharp.Compiler.Service.ProjectCrackerTool/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ module Program =
let addMSBuildv14BackupResolution () =
let onResolveEvent = new ResolveEventHandler(fun sender evArgs ->
let requestedAssembly = AssemblyName(evArgs.Name)
if requestedAssembly.Name.StartsWith("Microsoft.Build") &&
not (requestedAssembly.Name.EndsWith(".resources")) &&
if requestedAssembly.Name.StartsWith("Microsoft.Build", StringComparison.Ordinal) &&
not (requestedAssembly.Name.EndsWith(".resources", StringComparison.Ordinal)) &&
not (requestedAssembly.Version.ToString().Contains("12.0.0.0"))
then
// If the version of MSBuild that we're using wasn't present on the machine, then
Expand Down
5 changes: 2 additions & 3 deletions src/absil/il.fs
Original file line number Diff line number Diff line change
Expand Up @@ -362,14 +362,13 @@ let compareVersions x y =
else 0

let isMscorlib data =
if System.String.Compare(data.assemRefName, "mscorlib") = 0 then true
else false
data.assemRefName = "mscorlib"

let GetReferenceUnifiedVersion data =
let mutable highest = data.assemRefVersion
if not (isMscorlib data) then
for ref in AssemblyRefUniqueStampGenerator.Table do
if System.String.Compare(ref.assemRefName, data.assemRefName) = 0 && highest < ref.assemRefVersion then
if ref.assemRefName = data.assemRefName && highest < ref.assemRefVersion then
highest <- ref.assemRefVersion
highest

Expand Down
4 changes: 2 additions & 2 deletions src/absil/illib.fs
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ module String =
let (|StartsWith|_|) pattern value =
if String.IsNullOrWhiteSpace value then
None
elif value.StartsWith pattern then
elif value.StartsWith(pattern, StringComparison.Ordinal) then
Some()
else None

Expand All @@ -556,7 +556,7 @@ module String =
while not (isNull !line) do
yield !line
line := reader.ReadLine()
if str.EndsWith("\n") then
if str.EndsWith("\n", StringComparison.Ordinal) then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add StartsWithOrdinal/EndsWithOrdinal extension methods to illib.fs? Then it's easy to search and remove uses of StartsWith

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// last trailing space not returned
// http://stackoverflow.com/questions/19365404/stringreader-omits-trailing-linebreak
yield String.Empty
Expand Down
25 changes: 13 additions & 12 deletions src/fsharp/CompileOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ let OutputPhasedErrorR (os:StringBuilder) (err:PhasedDiagnostic) =
| ContextInfo.TupleInRecordFields ->
os.Append(ErrorFromAddingTypeEquation1E().Format t2 t1 tpcs) |> ignore
os.Append(System.Environment.NewLine + FSComp.SR.commaInsteadOfSemicolonInRecord()) |> ignore
| _ when t2 = "bool" && t1.EndsWith " ref" ->
| _ when t2 = "bool" && t1.EndsWith(" ref", StringComparison.Ordinal) ->
os.Append(ErrorFromAddingTypeEquation1E().Format t2 t1 tpcs) |> ignore
os.Append(System.Environment.NewLine + FSComp.SR.derefInsteadOfNot()) |> ignore
| _ -> os.Append(ErrorFromAddingTypeEquation1E().Format t2 t1 tpcs) |> ignore
Expand Down Expand Up @@ -1603,7 +1603,7 @@ let SanitizeFileName fileName implicitIncludeDir =
let currentDir = implicitIncludeDir

// if the file name is not rooted in the current directory, return the full path
if not(fullPath.StartsWith(currentDir)) then
if not(fullPath.StartsWith(currentDir, StringComparison.Ordinal)) then
fullPath
// if the file name is rooted in the current directory, return the relative path
else
Expand Down Expand Up @@ -1795,7 +1795,7 @@ type private TypeInThisAssembly = class end
let GetDefaultSystemValueTupleReference () =
try
let asm = typeof<System.ValueTuple<int, int>>.Assembly
if asm.FullName.StartsWith "System.ValueTuple" then
if asm.FullName.StartsWith("System.ValueTuple", StringComparison.Ordinal) then
Some asm.Location
else
let location = Path.GetDirectoryName(typeof<TypeInThisAssembly>.Assembly.Location)
Expand Down Expand Up @@ -3752,28 +3752,29 @@ type TcAssemblyResolutions(tcConfig: TcConfig, results: AssemblyResolution list,
//--------------------------------------------------------------------------

let IsSignatureDataResource (r: ILResource) =
r.Name.StartsWith FSharpSignatureDataResourceName ||
r.Name.StartsWith FSharpSignatureDataResourceName2
r.Name.StartsWith(FSharpSignatureDataResourceName, StringComparison.Ordinal) ||
r.Name.StartsWith(FSharpSignatureDataResourceName2, StringComparison.Ordinal)

let IsOptimizationDataResource (r: ILResource) =
r.Name.StartsWith FSharpOptimizationDataResourceName ||
r.Name.StartsWith FSharpOptimizationDataResourceName2
r.Name.StartsWith(FSharpOptimizationDataResourceName, StringComparison.Ordinal)||
r.Name.StartsWith(FSharpOptimizationDataResourceName2, StringComparison.Ordinal)

let GetSignatureDataResourceName (r: ILResource) =
if r.Name.StartsWith FSharpSignatureDataResourceName then
if r.Name.StartsWith(FSharpSignatureDataResourceName, StringComparison.Ordinal) then
String.dropPrefix r.Name FSharpSignatureDataResourceName
elif r.Name.StartsWith FSharpSignatureDataResourceName2 then
elif r.Name.StartsWith(FSharpSignatureDataResourceName2, StringComparison.Ordinal) then
String.dropPrefix r.Name FSharpSignatureDataResourceName2
else failwith "GetSignatureDataResourceName"

let GetOptimizationDataResourceName (r: ILResource) =
if r.Name.StartsWith FSharpOptimizationDataResourceName then
if r.Name.StartsWith(FSharpOptimizationDataResourceName, StringComparison.Ordinal) then
String.dropPrefix r.Name FSharpOptimizationDataResourceName
elif r.Name.StartsWith FSharpOptimizationDataResourceName2 then
elif r.Name.StartsWith(FSharpOptimizationDataResourceName2, StringComparison.Ordinal) then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know what comparison resource and assembly names should use. My understanding is that it doesn't actually make a difference if the thing being compared with is ASCII

String.dropPrefix r.Name FSharpOptimizationDataResourceName2
else failwith "GetOptimizationDataResourceName"

let IsReflectedDefinitionsResource (r: ILResource) = r.Name.StartsWith QuotationPickler.SerializedReflectedDefinitionsResourceNameBase
let IsReflectedDefinitionsResource (r: ILResource) =
r.Name.StartsWith(QuotationPickler.SerializedReflectedDefinitionsResourceNameBase, StringComparison.Ordinal)

let MakeILResource rname bytes =
{ Name = rname
Expand Down
6 changes: 3 additions & 3 deletions src/fsharp/CompileOptions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ module ResponseFile =
let parseLine (l: string) =
match l with
| s when String.IsNullOrWhiteSpace(s) -> None
| s when l.StartsWith("#") -> Some (ResponseFileLine.Comment (s.TrimStart('#')))
| s when l.StartsWith("#", StringComparison.Ordinal) -> Some (ResponseFileLine.Comment (s.TrimStart('#')))
| s -> Some (ResponseFileLine.CompilerOptionSpec (s.Trim()))

try
Expand Down Expand Up @@ -224,7 +224,7 @@ let ParseCompilerOptions (collectOtherArgument : string -> unit, blocks: Compile
if opt.Length = 2 || isSlashOpt opt then
opt <- opt.[1 ..]
// else, it should be a non-abbreviated option starting with "--"
elif opt.Length > 3 && opt.StartsWith("--") then
elif opt.Length > 3 && opt.StartsWith("--", StringComparison.Ordinal) then
opt <- opt.[2 ..]
else
opt <- ""
Expand Down Expand Up @@ -259,7 +259,7 @@ let ParseCompilerOptions (collectOtherArgument : string -> unit, blocks: Compile
let rec processArg args =
match args with
| [] -> ()
| ((rsp: string) :: t) when rsp.StartsWith("@") ->
| ((rsp: string) :: t) when rsp.StartsWith("@", StringComparison.Ordinal) ->
let responseFileOptions =
let fullpath =
try
Expand Down
6 changes: 3 additions & 3 deletions src/fsharp/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@

module internal Microsoft.FSharp.Compiler.ConstraintSolver

open System
open Internal.Utilities.Collections

open Microsoft.FSharp.Compiler
open Microsoft.FSharp.Compiler.AbstractIL
open Microsoft.FSharp.Compiler.AbstractIL.Internal.Library
Expand Down Expand Up @@ -1216,8 +1216,8 @@ and SolveMemberConstraint (csenv:ConstraintSolverEnv) ignoreUnresolvedOverload p

// First look for a solution by a record property
let recdPropSearch =
let isGetProp = nm.StartsWith "get_"
let isSetProp = nm.StartsWith "set_"
let isGetProp = nm.StartsWith("get_", StringComparison.Ordinal)
let isSetProp = nm.StartsWith("set_", StringComparison.Ordinal)
if argtys.IsEmpty && isGetProp || isSetProp then
let propName = nm.[4..]
let props =
Expand Down
16 changes: 9 additions & 7 deletions src/fsharp/ErrorResolutionHints.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/// Functions to format error message details
module internal Microsoft.FSharp.Compiler.ErrorResolutionHints

open System
open Internal.Utilities

let maxSuggestions = 5
Expand All @@ -28,17 +29,18 @@ let FilterPredictions (idText:string) (suggestionF:ErrorLogger.Suggestions) =
let allSuggestions = suggestionF()

let demangle (nm:string) =
if nm.StartsWith "( " && nm.EndsWith " )" then
if nm.StartsWith("( ", StringComparison.Ordinal) && nm.EndsWith(" )", StringComparison.Ordinal) then
let cleanName = nm.[2..nm.Length - 3]
cleanName
else nm

/// Returns `true` if given string is an operator display name, e.g. ( |>> )
let IsOperatorName (name: string) =
if not (name.StartsWith "( " && name.EndsWith " )") then false else
let name = name.[2..name.Length - 3]
let res = name |> Seq.forall (fun c -> c <> ' ')
res
if not (name.StartsWith("( ", StringComparison.Ordinal) && name.EndsWith(" )", StringComparison.Ordinal)) then
false
else
let name = name.[2..name.Length - 3]
name |> Seq.forall (fun c -> c <> ' ')

if allSuggestions.Contains idText then [] else // some other parsing error occurred
allSuggestions
Expand All @@ -47,11 +49,11 @@ let FilterPredictions (idText:string) (suggestionF:ErrorLogger.Suggestions) =
// 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.StartsWith "_" then None else
if IsOperatorName suggestion || suggestion.StartsWith("_", StringComparison.Ordinal) then None else
let suggestion:string = demangle suggestion
let suggestedText = suggestion.ToUpperInvariant()
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
if similarity >= highConfidenceThreshold || suggestion.EndsWith ("." + idText) then
if similarity >= highConfidenceThreshold || suggestion.EndsWith("." + idText, StringComparison.Ordinal) then
Some(similarity, suggestion)
elif similarity < minThresholdForSuggestions && suggestedText.Length > minStringLengthForThreshold then
None
Expand Down
5 changes: 4 additions & 1 deletion src/fsharp/MethodCalls.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/// Logic associated with resolving method calls.
module internal Microsoft.FSharp.Compiler.MethodCalls

open System
open Microsoft.FSharp.Compiler
open Microsoft.FSharp.Compiler.AbstractIL.IL
open Microsoft.FSharp.Compiler.AbstractIL.Internal.Library
Expand Down Expand Up @@ -1260,7 +1261,9 @@ let MethInfoChecks g amap isInstance tyargsOpt objArgs ad m (minfo:MethInfo) =
if not (IsTypeAndMethInfoAccessible amap m adOriginal ad minfo) then
error (Error (FSComp.SR.tcMethodNotAccessible(minfo.LogicalName), m))

if isAnyTupleTy g minfo.ApparentEnclosingType && not minfo.IsExtensionMember && (minfo.LogicalName.StartsWith "get_Item" || minfo.LogicalName.StartsWith "get_Rest") then
if isAnyTupleTy g minfo.ApparentEnclosingType && not minfo.IsExtensionMember &&
(minfo.LogicalName.StartsWith("get_Item", StringComparison.Ordinal) ||
minfo.LogicalName.StartsWith("get_Rest", StringComparison.Ordinal)) then
warning (Error (FSComp.SR.tcTupleMemberNotNormallyUsed(), m))

CheckMethInfoAttributes g m tyargsOpt minfo |> CommitOperationResult
Expand Down
7 changes: 4 additions & 3 deletions src/fsharp/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
/// Name environment and name resolution
module internal Microsoft.FSharp.Compiler.NameResolution

open System
open Internal.Utilities
open Microsoft.FSharp.Compiler
open Microsoft.FSharp.Compiler.Range
Expand Down Expand Up @@ -2872,7 +2873,7 @@ let rec ResolveTypeLongIdentPrim sink (ncenv:NameResolver) occurence first fully
| ItemOccurence.UseInAttribute ->
[yield e.Value.DisplayName
yield e.Value.DemangledModuleOrNamespaceName
if e.Value.DisplayName.EndsWith "Attribute" then
if e.Value.DisplayName.EndsWith("Attribute", StringComparison.Ordinal) then
yield e.Value.DisplayName.Replace("Attribute","")]
| _ -> [e.Value.DisplayName; e.Value.DemangledModuleOrNamespaceName])
|> HashSet
Expand Down Expand Up @@ -3580,7 +3581,7 @@ let ResolveCompletionsInType (ncenv: NameResolver) nenv (completionTargets: Reso
if methsWithStaticParams.IsEmpty then minfos
else minfos |> List.filter (fun minfo ->
let nm = minfo.LogicalName
not (nm.Contains "," && methsWithStaticParams |> List.exists (fun m -> nm.StartsWith(m))))
not (nm.Contains "," && methsWithStaticParams |> List.exists (fun m -> nm.StartsWith(m, StringComparison.Ordinal))))
#endif

minfos
Expand Down Expand Up @@ -4209,7 +4210,7 @@ let ResolveCompletionsInTypeForItem (ncenv: NameResolver) nenv m ad statics typ
if methsWithStaticParams.IsEmpty then minfos
else minfos |> List.filter (fun minfo ->
let nm = minfo.LogicalName
not (nm.Contains "," && methsWithStaticParams |> List.exists (fun m -> nm.StartsWith(m))))
not (nm.Contains "," && methsWithStaticParams |> List.exists (fun m -> nm.StartsWith(m, StringComparison.Ordinal))))
#endif

minfos
Expand Down
5 changes: 3 additions & 2 deletions src/fsharp/NicePrint.fs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

module internal Microsoft.FSharp.Compiler.NicePrint

open System
open Microsoft.FSharp.Compiler.AbstractIL
open Microsoft.FSharp.Compiler.AbstractIL.IL
open Microsoft.FSharp.Compiler.AbstractIL.Internal
Expand Down Expand Up @@ -71,7 +72,7 @@ module internal PrintUtilities =
tcref.DisplayName // has no static params
else
tcref.DisplayName+"<...>" // shorten
if isAttribute && name.EndsWith "Attribute" then
if isAttribute && name.EndsWith("Attribute", StringComparison.Ordinal) then
String.dropSuffix name "Attribute"
else
name
Expand Down Expand Up @@ -655,7 +656,7 @@ module private PrintTypes =
| ILAttrib ilMethRef ->
let trimmedName =
let name = ilMethRef.DeclaringTypeRef.Name
if name.EndsWith "Attribute" then
if name.EndsWith("Attribute", StringComparison.Ordinal) then
String.dropSuffix name "Attribute"
else
name
Expand Down
5 changes: 3 additions & 2 deletions src/fsharp/PostInferenceChecks.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
/// is complete.
module internal Microsoft.FSharp.Compiler.PostTypeCheckSemanticChecks

open System
open System.Collections.Generic

open Microsoft.FSharp.Compiler
Expand Down Expand Up @@ -1309,14 +1310,14 @@ let CheckModuleBinding cenv env (TBind(v,e,_) as bind) =

// Default augmentation contains the nasty 'Case<UnionCase>' etc.
let prefix = "New"
if nm.StartsWith prefix then
if nm.StartsWith(prefix, StringComparison.Ordinal) then
match tcref.GetUnionCaseByName(nm.[prefix.Length ..]) with
| Some(uc) -> error(NameClash(nm,kind,v.DisplayName,v.Range, FSComp.SR.chkUnionCaseCompiledForm(),uc.DisplayName,uc.Range))
| None -> ()

// Default augmentation contains the nasty 'Is<UnionCase>' etc.
let prefix = "Is"
if nm.StartsWith prefix && hasDefaultAugmentation then
if nm.StartsWith(prefix, StringComparison.Ordinal) && hasDefaultAugmentation then
match tcref.GetUnionCaseByName(nm.[prefix.Length ..]) with
| Some(uc) -> error(NameClash(nm,kind,v.DisplayName,v.Range, FSComp.SR.chkUnionCaseDefaultAugmentation(),uc.DisplayName,uc.Range))
| None -> ()
Expand Down
12 changes: 7 additions & 5 deletions src/fsharp/PrettyNaming.fs
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,15 @@ module public Microsoft.FSharp.Compiler.PrettyNaming

/// Returns `true` if given string is an operator display name, e.g. ( |>> )
let IsOperatorName (name: string) =
let name = if name.StartsWith "( " && name.EndsWith " )" then name.[2..name.Length - 3] else name
let name =
if name.StartsWith("( ", StringComparison.Ordinal) && name.EndsWith(" )", StringComparison.Ordinal) then
name.[2 .. name.Length - 3]
else name
// there is single operator containing a space - range operator with step: `.. ..`
let res = name = ".. .." || name |> Seq.forall (fun c -> opCharSet.Contains c && c <> ' ')
res
name = ".. .." || name |> Seq.forall (fun c -> opCharSet.Contains c && c <> ' ')

let IsMangledOpName (n:string) =
n.StartsWith (opNamePrefix, StringComparison.Ordinal)
n.StartsWith(opNamePrefix, StringComparison.Ordinal)

/// Compiles a custom operator into a mangled operator name.
/// For example, "!%" becomes "op_DereferencePercent".
Expand Down Expand Up @@ -468,7 +470,7 @@ module public Microsoft.FSharp.Compiler.PrettyNaming
if IsCompilerGeneratedName nm then nm else nm+compilerGeneratedMarker

let GetBasicNameOfPossibleCompilerGeneratedName (name:string) =
match name.IndexOf compilerGeneratedMarker with
match name.IndexOf(compilerGeneratedMarker, StringComparison.Ordinal) with
| -1 | 0 -> name
| n -> name.[0..n-1]

Expand Down
5 changes: 3 additions & 2 deletions src/fsharp/QuotationTranslator.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

module internal Microsoft.FSharp.Compiler.QuotationTranslator

open System
open System.Collections.Generic
open Internal.Utilities
open Microsoft.FSharp.Compiler
open Microsoft.FSharp.Compiler.AbstractIL.IL
Expand All @@ -15,7 +17,6 @@ open Microsoft.FSharp.Compiler.PrettyNaming
open Microsoft.FSharp.Compiler.ErrorLogger
open Microsoft.FSharp.Compiler.TcGlobals
open Microsoft.FSharp.Compiler.Range
open System.Collections.Generic

module QP = Microsoft.FSharp.Compiler.QuotationPickler

Expand Down Expand Up @@ -161,7 +162,7 @@ let (|ObjectInitializationCheck|_|) g expr =
[| TTarget([], Expr.App(Expr.Val(failInitRef, _, _), _, _, _, _), _); _ |], _, resultTy
) when
IsCompilerGeneratedName name &&
name.StartsWith "init" &&
name.StartsWith("init", StringComparison.Ordinal) &&
selfRef.BaseOrThisInfo = MemberThisVal &&
valRefEq g failInitRef (ValRefForIntrinsic g.fail_init_info) &&
isUnitTy g resultTy -> Some()
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/SimulatedMSBuildReferenceResolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ let internal SimulatedMSBuildResolver =
#if !FX_RESHAPED_MSBUILD
// For this one we need to get the version search exactly right, without doing a load
try
if not found && r.StartsWith("FSharp.Core, Version=") && Environment.OSVersion.Platform = PlatformID.Win32NT then
if not found && r.StartsWith("FSharp.Core, Version=", StringComparison.Ordinal) && Environment.OSVersion.Platform = PlatformID.Win32NT then
let n = AssemblyName(r)
let fscoreDir0 =
let PF =
Expand Down
Loading