Skip to content

Commit cc29923

Browse files
CopilotT-Gro
andcommitted
Refactor to use immutable LanguageVersion with reflection-based feature parsing
- Changed LanguageVersion to use immutable array of disabled features instead of mutable set - Added optional constructor parameter for disabled features array - Added WithDisabledFeatures method that returns a new LanguageVersion instance - Replaced manual feature name mapping with reflection-based TryParseFeature using BindingFlags.NonPublic - Updated CompilerOptions to use immutable pattern with Array.append - Changed disabledLanguageFeatures in TcConfigBuilder from Set to array - Updated tests to use typecheck instead of compile and assert exact error codes - Applied code formatting Addresses feedback from @T-Gro about making the design immutable and using reflection. Co-authored-by: T-Gro <[email protected]>
1 parent 2852d9c commit cc29923

File tree

6 files changed

+36
-122
lines changed

6 files changed

+36
-122
lines changed

src/Compiler/Driver/CompilerConfig.fs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ type TcConfigBuilder =
644644

645645
mutable langVersion: LanguageVersion
646646

647-
mutable disabledLanguageFeatures: Set<LanguageFeature>
647+
mutable disabledLanguageFeatures: LanguageFeature array
648648

649649
mutable xmlDocInfoLoader: IXmlDocumentationInfoLoader option
650650

@@ -838,7 +838,7 @@ type TcConfigBuilder =
838838
pathMap = PathMap.empty
839839
applyLineDirectives = true
840840
langVersion = LanguageVersion.Default
841-
disabledLanguageFeatures = Set.empty
841+
disabledLanguageFeatures = [||]
842842
implicitIncludeDir = implicitIncludeDir
843843
defaultFSharpBinariesDir = defaultFSharpBinariesDir
844844
reduceMemoryUsage = reduceMemoryUsage

src/Compiler/Driver/CompilerConfig.fsi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ type TcConfigBuilder =
512512

513513
mutable langVersion: LanguageVersion
514514

515-
mutable disabledLanguageFeatures: Set<LanguageFeature>
515+
mutable disabledLanguageFeatures: LanguageFeature array
516516

517517
mutable xmlDocInfoLoader: IXmlDocumentationInfoLoader option
518518

src/Compiler/Driver/CompilerOptions.fs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,8 +1178,9 @@ let languageFlags tcConfigB =
11781178
"langversion",
11791179
tagLangVersionValues,
11801180
OptionString(fun switch ->
1181-
tcConfigB.langVersion <- setLanguageVersion switch
1182-
tcConfigB.langVersion.SetDisabledFeatures(tcConfigB.disabledLanguageFeatures)),
1181+
let newVersion = setLanguageVersion switch
1182+
// Preserve disabled features when updating version
1183+
tcConfigB.langVersion <- newVersion.WithDisabledFeatures(tcConfigB.disabledLanguageFeatures)),
11831184
None,
11841185
Some(FSComp.SR.optsSetLangVersion ())
11851186
)
@@ -1191,8 +1192,8 @@ let languageFlags tcConfigB =
11911192
OptionStringList(fun featureName ->
11921193
match LanguageVersion.TryParseFeature(featureName) with
11931194
| Some feature ->
1194-
tcConfigB.disabledLanguageFeatures <- tcConfigB.disabledLanguageFeatures.Add(feature)
1195-
tcConfigB.langVersion.SetDisabledFeatures(tcConfigB.disabledLanguageFeatures)
1195+
tcConfigB.disabledLanguageFeatures <- Array.append tcConfigB.disabledLanguageFeatures [| feature |]
1196+
tcConfigB.langVersion <- tcConfigB.langVersion.WithDisabledFeatures(tcConfigB.disabledLanguageFeatures)
11961197
| None -> error (Error(FSComp.SR.optsUnrecognizedLanguageFeature featureName, rangeCmdArgs))),
11971198
None,
11981199
Some(FSComp.SR.optsDisableLanguageFeature ())

src/Compiler/Facilities/LanguageFeatures.fs

Lines changed: 15 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ type LanguageFeature =
107107
| ReturnFromFinal
108108

109109
/// LanguageVersion management
110-
type LanguageVersion(versionText) =
110+
type LanguageVersion(versionText, ?disabledFeaturesArray: LanguageFeature array) =
111111

112112
// When we increment language versions here preview is higher than current RTM version
113113
static let languageVersion46 = 4.6m
@@ -279,19 +279,19 @@ type LanguageVersion(versionText) =
279279

280280
let specifiedString = versionToString specified
281281

282-
let mutable disabledFeatures: Set<LanguageFeature> = Set.empty
282+
let disabledFeatures: LanguageFeature array = defaultArg disabledFeaturesArray [||]
283283

284284
/// Check if this feature is supported by the selected langversion
285285
member _.SupportsFeature featureId =
286-
if disabledFeatures.Contains featureId then
286+
if Array.contains featureId disabledFeatures then
287287
false
288288
else
289289
match features.TryGetValue featureId with
290290
| true, v -> v <= specified
291291
| false, _ -> false
292292

293-
/// Set the disabled features for this language version
294-
member _.SetDisabledFeatures(disabled: Set<LanguageFeature>) = disabledFeatures <- disabled
293+
/// Create a new LanguageVersion with updated disabled features
294+
member _.WithDisabledFeatures(disabled: LanguageFeature array) = LanguageVersion(versionText, disabled)
295295

296296
/// Has preview been explicitly specified
297297
member _.IsExplicitlySpecifiedAs50OrBefore() =
@@ -430,103 +430,17 @@ type LanguageVersion(versionText) =
430430
| true, v -> versionToString v
431431
| _ -> invalidArg "feature" "Internal error: Unable to find feature."
432432

433-
/// Try to parse a feature name string to a LanguageFeature option
433+
/// Try to parse a feature name string to a LanguageFeature option using reflection
434434
static member TryParseFeature(featureName: string) =
435-
let normalized = featureName.Trim().ToLowerInvariant()
436-
437-
match normalized with
438-
| "singleunderscorepattern" -> Some LanguageFeature.SingleUnderscorePattern
439-
| "wildcardinforloop" -> Some LanguageFeature.WildCardInForLoop
440-
| "relaxwhitespace" -> Some LanguageFeature.RelaxWhitespace
441-
| "relaxwhitespace2" -> Some LanguageFeature.RelaxWhitespace2
442-
| "strictindentation" -> Some LanguageFeature.StrictIndentation
443-
| "nameof" -> Some LanguageFeature.NameOf
444-
| "implicityield" -> Some LanguageFeature.ImplicitYield
445-
| "opentypedeclaration" -> Some LanguageFeature.OpenTypeDeclaration
446-
| "dotlessfloat32literal" -> Some LanguageFeature.DotlessFloat32Literal
447-
| "packagemanagement" -> Some LanguageFeature.PackageManagement
448-
| "fromendslicing" -> Some LanguageFeature.FromEndSlicing
449-
| "fixedindexslice3d4d" -> Some LanguageFeature.FixedIndexSlice3d4d
450-
| "andbang" -> Some LanguageFeature.AndBang
451-
| "resumablestatemachines" -> Some LanguageFeature.ResumableStateMachines
452-
| "nullableoptionalinterop" -> Some LanguageFeature.NullableOptionalInterop
453-
| "defaultinterfacememberconsumption" -> Some LanguageFeature.DefaultInterfaceMemberConsumption
454-
| "witnesspassing" -> Some LanguageFeature.WitnessPassing
455-
| "additionaltypedirectedconversions" -> Some LanguageFeature.AdditionalTypeDirectedConversions
456-
| "interfaceswithmultiplegenericinstantiation" -> Some LanguageFeature.InterfacesWithMultipleGenericInstantiation
457-
| "stringinterpolation" -> Some LanguageFeature.StringInterpolation
458-
| "overloadsforcustomoperations" -> Some LanguageFeature.OverloadsForCustomOperations
459-
| "expandedmeasurables" -> Some LanguageFeature.ExpandedMeasurables
460-
| "nullnesschecking" -> Some LanguageFeature.NullnessChecking
461-
| "structactivepattern" -> Some LanguageFeature.StructActivePattern
462-
| "printfbinaryformat" -> Some LanguageFeature.PrintfBinaryFormat
463-
| "indexernotationwithoutdot" -> Some LanguageFeature.IndexerNotationWithoutDot
464-
| "refcellnotationinformationals" -> Some LanguageFeature.RefCellNotationInformationals
465-
| "usebindingvaluediscard" -> Some LanguageFeature.UseBindingValueDiscard
466-
| "unionispropertiesvisible" -> Some LanguageFeature.UnionIsPropertiesVisible
467-
| "nonvariablepatternstorightofaspatterns" -> Some LanguageFeature.NonVariablePatternsToRightOfAsPatterns
468-
| "attributestorightofmodulekeyword" -> Some LanguageFeature.AttributesToRightOfModuleKeyword
469-
| "mlcompatrevisions" -> Some LanguageFeature.MLCompatRevisions
470-
| "betterexceptionprinting" -> Some LanguageFeature.BetterExceptionPrinting
471-
| "delegatetypenameresolutionfix" -> Some LanguageFeature.DelegateTypeNameResolutionFix
472-
| "reallylonglists" -> Some LanguageFeature.ReallyLongLists
473-
| "errorondeprecatedrequirequalifiedaccess" -> Some LanguageFeature.ErrorOnDeprecatedRequireQualifiedAccess
474-
| "requiredpropertiessupport" -> Some LanguageFeature.RequiredPropertiesSupport
475-
| "initpropertiessupport" -> Some LanguageFeature.InitPropertiesSupport
476-
| "lowercaseduwhenrequirequalifiedaccess" -> Some LanguageFeature.LowercaseDUWhenRequireQualifiedAccess
477-
| "interfaceswithabstractstaticmembers" -> Some LanguageFeature.InterfacesWithAbstractStaticMembers
478-
| "selftypeconstraints" -> Some LanguageFeature.SelfTypeConstraints
479-
| "accessorfunctionshorthand" -> Some LanguageFeature.AccessorFunctionShorthand
480-
| "matchnotallowedforunioncasewithnodata" -> Some LanguageFeature.MatchNotAllowedForUnionCaseWithNoData
481-
| "csharpextensionattributenotrequired" -> Some LanguageFeature.CSharpExtensionAttributeNotRequired
482-
| "errorfornonvirtualmembersoverrides" -> Some LanguageFeature.ErrorForNonVirtualMembersOverrides
483-
| "warningwheninliningmethodimplnoinlinemarkedfunction" -> Some LanguageFeature.WarningWhenInliningMethodImplNoInlineMarkedFunction
484-
| "escapedotnetformattablestrings" -> Some LanguageFeature.EscapeDotnetFormattableStrings
485-
| "arithmeticinliterals" -> Some LanguageFeature.ArithmeticInLiterals
486-
| "errorreportingonstaticclasses" -> Some LanguageFeature.ErrorReportingOnStaticClasses
487-
| "trywithinseqexpression" -> Some LanguageFeature.TryWithInSeqExpression
488-
| "warningwhencopyandupdaterecordchangesallfields" -> Some LanguageFeature.WarningWhenCopyAndUpdateRecordChangesAllFields
489-
| "staticmembersininterfaces" -> Some LanguageFeature.StaticMembersInInterfaces
490-
| "noninlineliteralsasprintfformat" -> Some LanguageFeature.NonInlineLiteralsAsPrintfFormat
491-
| "nestedcopyandupdate" -> Some LanguageFeature.NestedCopyAndUpdate
492-
| "extendedstringinterpolation" -> Some LanguageFeature.ExtendedStringInterpolation
493-
| "warningwhenmultiplerecdtypechoice" -> Some LanguageFeature.WarningWhenMultipleRecdTypeChoice
494-
| "improvedimpliedargumentnames" -> Some LanguageFeature.ImprovedImpliedArgumentNames
495-
| "diagnosticforobjinference" -> Some LanguageFeature.DiagnosticForObjInference
496-
| "constraintintersectiononflexibletypes" -> Some LanguageFeature.ConstraintIntersectionOnFlexibleTypes
497-
| "staticletinrecordsdustemptypes" -> Some LanguageFeature.StaticLetInRecordsDusEmptyTypes
498-
| "warningwhentailrecattributebutnontailrecusage" -> Some LanguageFeature.WarningWhenTailRecAttributeButNonTailRecUsage
499-
| "unmanagedconstraintcsharpinterop" -> Some LanguageFeature.UnmanagedConstraintCsharpInterop
500-
| "whilebang" -> Some LanguageFeature.WhileBang
501-
| "reusesamefieldssinstructunions" -> Some LanguageFeature.ReuseSameFieldsInStructUnions
502-
| "extendedfixedbindings" -> Some LanguageFeature.ExtendedFixedBindings
503-
| "preferstringgetpinnablereference" -> Some LanguageFeature.PreferStringGetPinnableReference
504-
| "preferextensionmethodoverplainproperty" -> Some LanguageFeature.PreferExtensionMethodOverPlainProperty
505-
| "warningindexedpropertiesgetsetsametype" -> Some LanguageFeature.WarningIndexedPropertiesGetSetSameType
506-
| "warningwhentailcallattrononrec" -> Some LanguageFeature.WarningWhenTailCallAttrOnNonRec
507-
| "booleanreturningandreturntypedirectedpartialactivepattern" ->
508-
Some LanguageFeature.BooleanReturningAndReturnTypeDirectedPartialActivePattern
509-
| "enforceattributetargets" -> Some LanguageFeature.EnforceAttributeTargets
510-
| "lowerinterpolatedstringtoconcat" -> Some LanguageFeature.LowerInterpolatedStringToConcat
511-
| "lowerintegralrangestofastloops" -> Some LanguageFeature.LowerIntegralRangesToFastLoops
512-
| "allowaccessmodifierstautopropertiesgettersandsetters" ->
513-
Some LanguageFeature.AllowAccessModifiersToAutoPropertiesGettersAndSetters
514-
| "lowersimplemappingsincomprehensionstofastloops" -> Some LanguageFeature.LowerSimpleMappingsInComprehensionsToFastLoops
515-
| "parsedhashdirectiveargumentnonquotes" -> Some LanguageFeature.ParsedHashDirectiveArgumentNonQuotes
516-
| "emptybodiedcomputationexpressions" -> Some LanguageFeature.EmptyBodiedComputationExpressions
517-
| "allowobjectexpressionwithoutoverrides" -> Some LanguageFeature.AllowObjectExpressionWithoutOverrides
518-
| "dontwarnunuppercaseidentifiersinbindingpatterns" -> Some LanguageFeature.DontWarnOnUppercaseIdentifiersInBindingPatterns
519-
| "usetypesubsumptioncache" -> Some LanguageFeature.UseTypeSubsumptionCache
520-
| "deprecateplaceswheresecanbemitted" -> Some LanguageFeature.DeprecatePlacesWhereSeqCanBeOmitted
521-
| "supportvalueoptionsasoptionalparameters" -> Some LanguageFeature.SupportValueOptionsAsOptionalParameters
522-
| "warnwhenunitpassedtoobjarg" -> Some LanguageFeature.WarnWhenUnitPassedToObjArg
523-
| "usebangbindingvaluediscard" -> Some LanguageFeature.UseBangBindingValueDiscard
524-
| "betteranonymousrecordparsing" -> Some LanguageFeature.BetterAnonymousRecordParsing
525-
| "scopednowarn" -> Some LanguageFeature.ScopedNowarn
526-
| "erroroninvaliddeclsintypedefinitions" -> Some LanguageFeature.ErrorOnInvalidDeclsInTypeDefinitions
527-
| "allowtypedletuseandbang" -> Some LanguageFeature.AllowTypedLetUseAndBang
528-
| "returnfromfinal" -> Some LanguageFeature.ReturnFromFinal
529-
| _ -> None
435+
let normalized = featureName.Trim()
436+
437+
Microsoft.FSharp.Reflection.FSharpType.GetUnionCases(
438+
typeof<LanguageFeature>,
439+
System.Reflection.BindingFlags.Public
440+
||| System.Reflection.BindingFlags.NonPublic
441+
)
442+
|> Array.tryFind (fun case -> System.String.Equals(case.Name, normalized, System.StringComparison.OrdinalIgnoreCase))
443+
|> Option.map (fun case -> Microsoft.FSharp.Reflection.FSharpValue.MakeUnion(case, [||]) :?> LanguageFeature)
530444

531445
override x.Equals(yobj: obj) =
532446
match yobj with

src/Compiler/Facilities/LanguageFeatures.fsi

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ type LanguageFeature =
101101
type LanguageVersion =
102102

103103
/// Create a LanguageVersion management object
104-
new: string -> LanguageVersion
104+
new: string * ?disabledFeaturesArray: LanguageFeature array -> LanguageVersion
105105

106106
/// Get the list of valid versions
107107
static member ContainsVersion: string -> bool
@@ -115,8 +115,8 @@ type LanguageVersion =
115115
/// Does the selected LanguageVersion support the specified feature
116116
member SupportsFeature: LanguageFeature -> bool
117117

118-
/// Set the disabled features for this language version
119-
member SetDisabledFeatures: Set<LanguageFeature> -> unit
118+
/// Create a new LanguageVersion with updated disabled features
119+
member WithDisabledFeatures: LanguageFeature array -> LanguageVersion
120120

121121
/// Get the list of valid versions
122122
static member ValidVersions: string[]

tests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/disableLanguageFeature.fs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,12 @@ open FSharp.Test.Compiler
99
module disableLanguageFeature =
1010

1111
[<Fact>]
12-
let ``disableLanguageFeature with valid feature name should compile successfully``() =
12+
let ``disableLanguageFeature with valid feature name should typecheck successfully``() =
1313
FSharp """
1414
printfn "Hello, World"
1515
"""
16-
|> asExe
1716
|> withOptions ["--disableLanguageFeature:NameOf"]
18-
|> compile
17+
|> typecheck
1918
|> shouldSucceed
2019
|> ignore
2120

@@ -25,20 +24,20 @@ printfn "Hello, World"
2524
let x = 5
2625
let name = nameof(x)
2726
"""
28-
|> asExe
2927
|> withOptions ["--langversion:latest"; "--disableLanguageFeature:NameOf"]
30-
|> compile
28+
|> typecheck
3129
|> shouldFail
30+
|> withErrorCode 39
31+
|> withDiagnosticMessageMatches "The value or constructor 'nameof' is not defined"
3232
|> ignore
3333

3434
[<Fact>]
3535
let ``disableLanguageFeature with invalid feature name should fail``() =
3636
FSharp """
3737
printfn "Hello, World"
3838
"""
39-
|> asExe
4039
|> withOptions ["--disableLanguageFeature:InvalidFeatureName"]
41-
|> compile
40+
|> typecheck
4241
|> shouldFail
4342
|> withErrorCode 3879
4443
|> withDiagnosticMessageMatches "Unrecognized language feature name"
@@ -50,10 +49,10 @@ printfn "Hello, World"
5049
let x = 5
5150
let name = nameof(x)
5251
"""
53-
|> asExe
5452
|> withOptions ["--langversion:latest"; "--disableLanguageFeature:NameOf"; "--disableLanguageFeature:StringInterpolation"]
55-
|> compile
53+
|> typecheck
5654
|> shouldFail
55+
|> withErrorCode 39
5756
|> ignore
5857

5958
[<Fact>]
@@ -62,8 +61,8 @@ let name = nameof(x)
6261
let x = 5
6362
let name = nameof(x)
6463
"""
65-
|> asExe
6664
|> withOptions ["--langversion:latest"; "--disableLanguageFeature:nameof"]
67-
|> compile
65+
|> typecheck
6866
|> shouldFail
67+
|> withErrorCode 39
6968
|> ignore

0 commit comments

Comments
 (0)