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

Init-only & required properties support #13490

Merged
merged 22 commits into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9eedfb7
WIP: Added init-only properties error + added some tests
vzarytovskii Jul 11, 2022
e2d9d68
WIP: Separate tests
vzarytovskii Jul 11, 2022
2a05ed2
Added check for explicit setters
vzarytovskii Jul 13, 2022
8a7d48c
Fix tests
vzarytovskii Jul 13, 2022
bdcab6b
Forbit calling setters on the methods
vzarytovskii Jul 14, 2022
63fc2fb
wip: support required attributes
vzarytovskii Jul 14, 2022
b1ffb44
Added required properties support + apply fantomas
vzarytovskii Jul 15, 2022
bb8b77a
Merge branch 'main' into required-and-init
vzarytovskii Jul 15, 2022
4c7029f
Merge remote-tracking branch 'upstream/main' into required-and-init
vzarytovskii Jul 22, 2022
2681745
Moved compilerfeaturerequired check to a method
vzarytovskii Jul 22, 2022
81da22c
Updated
vzarytovskii Jul 22, 2022
662186f
fantomas
vzarytovskii Jul 22, 2022
005af55
Internalize empty
vzarytovskii Jul 22, 2022
5784869
Add setter check + add more unsupported attributes
vzarytovskii Jul 25, 2022
bf2ae5e
Merge remote-tracking branch 'upstream/main' into required-and-init
vzarytovskii Jul 25, 2022
cc928ef
Fix prop pages
vzarytovskii Jul 25, 2022
0ef72fd
Update src/Compiler/Checking/CheckExpressions.fs
vzarytovskii Jul 28, 2022
07762ca
Addressed PR comments + added additional tests
vzarytovskii Jul 28, 2022
4051bdc
Merge remote-tracking branch 'upstream/main' into required-and-init
vzarytovskii Jul 28, 2022
3bc625b
Update src/Compiler/Checking/infos.fs
vzarytovskii Jul 28, 2022
66407b2
Moved condition outside of list expression
vzarytovskii Jul 28, 2022
764c158
PR fixes
vzarytovskii Jul 28, 2022
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
17 changes: 9 additions & 8 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@
<SystemRuntimeCompilerServicesUnsafeVersion>6.0.0</SystemRuntimeCompilerServicesUnsafeVersion>
<SystemValueTupleVersion>4.5.0</SystemValueTupleVersion>
<!-- VisualStudio package versions -->
<VisualStudioImplementationPackagesVersion>17.2.178-preview</VisualStudioImplementationPackagesVersion>
<VisualStudioContractPackagesVersion>17.2.0-preview-1-32131-009</VisualStudioContractPackagesVersion>
<VisualStudioImplementationPackagesVersion>17.3.133-preview</VisualStudioImplementationPackagesVersion>
<VisualStudioContractPackagesVersion>17.3.0-preview-1-32407-044</VisualStudioContractPackagesVersion>
<VisualStudioProjectSystemPackagesVersion>17.0.77-pre-g62a6cb5699</VisualStudioProjectSystemPackagesVersion>
<MicrosoftVisualStudioInteropVersion>$(VisualStudioContractPackagesVersion)</MicrosoftVisualStudioInteropVersion>
<MicrosoftInternalVisualStudioInteropVersion>$(VisualStudioContractPackagesVersion)</MicrosoftInternalVisualStudioInteropVersion>
Expand All @@ -135,7 +135,7 @@
<EnvDTEVersion>$(VisualStudioContractPackagesVersion)</EnvDTEVersion>
<EnvDTE80Version>$(VisualStudioContractPackagesVersion)</EnvDTE80Version>
<!-- Roslyn packages -->
<RoslynVersion>4.2.0-3.22154.1</RoslynVersion>
<RoslynVersion>4.4.0-1.22360.2</RoslynVersion>
<MicrosoftCodeAnalysisEditorFeaturesVersion>$(RoslynVersion)</MicrosoftCodeAnalysisEditorFeaturesVersion>
<MicrosoftCodeAnalysisEditorFeaturesTextVersion>$(RoslynVersion)</MicrosoftCodeAnalysisEditorFeaturesTextVersion>
<MicrosoftCodeAnalysisEditorFeaturesWpfVersion>$(RoslynVersion)</MicrosoftCodeAnalysisEditorFeaturesWpfVersion>
Expand All @@ -144,8 +144,9 @@
<MicrosoftCodeAnalysisCSharpVersion>$(RoslynVersion)</MicrosoftCodeAnalysisCSharpVersion>
<MicrosoftCodeAnalysisTestResourcesProprietaryVersion>2.0.28</MicrosoftCodeAnalysisTestResourcesProprietaryVersion>
<MicrosoftVisualStudioLanguageServicesVersion>$(RoslynVersion)</MicrosoftVisualStudioLanguageServicesVersion>

<!-- Microsoft Build packages -->
<MicrosoftBuildOverallPackagesVersion>17.0.0</MicrosoftBuildOverallPackagesVersion>
<MicrosoftBuildOverallPackagesVersion>17.1.0</MicrosoftBuildOverallPackagesVersion>
<MicrosoftBuildVersion>$(MicrosoftBuildOverallPackagesVersion)</MicrosoftBuildVersion>
<MicrosoftBuildFrameworkVersion>$(MicrosoftBuildOverallPackagesVersion)</MicrosoftBuildFrameworkVersion>
<MicrosoftBuildTasksCoreVersion>$(MicrosoftBuildOverallPackagesVersion)</MicrosoftBuildTasksCoreVersion>
Expand All @@ -169,7 +170,7 @@
<MicrosoftVisualStudioShellFrameworkVersion>$(VisualStudioContractPackagesVersion)</MicrosoftVisualStudioShellFrameworkVersion>
<MicrosoftVisualStudioPackageLanguageService150Version>$(VisualStudioContractPackagesVersion)</MicrosoftVisualStudioPackageLanguageService150Version>
<!-- Misc. Visual Studio packages -->
<MicrosoftVisualStudioRpcContractsVersion>17.2.22-alpha</MicrosoftVisualStudioRpcContractsVersion>
<MicrosoftVisualStudioRpcContractsVersion>17.3.3-alpha</MicrosoftVisualStudioRpcContractsVersion>
<MicrosoftVisualStudioComponentModelHostVersion>$(VisualStudioImplementationPackagesVersion)</MicrosoftVisualStudioComponentModelHostVersion>
<MicrosoftVisualFSharpMicrosoftVisualStudioShellUIInternalVersion>17.0.0</MicrosoftVisualFSharpMicrosoftVisualStudioShellUIInternalVersion>
<MicrosoftVisualStudioDesignerInterfacesVersion>$(VisualStudioContractPackagesVersion)</MicrosoftVisualStudioDesignerInterfacesVersion>
Expand All @@ -185,9 +186,9 @@
<MicrosoftVisualStudioShellImmutable150Version>15.0.25123-Dev15Preview</MicrosoftVisualStudioShellImmutable150Version>
<MicrosoftVisualStudioShellInterop160DesignTimeVersion>16.0.1</MicrosoftVisualStudioShellInterop160DesignTimeVersion>
<MicrosoftVisualStudioShellInterop16DesignTimeVersion>16.0.28924.11111</MicrosoftVisualStudioShellInterop16DesignTimeVersion>
<MicrosoftVisualStudioThreadingVersion>17.2.10-alpha</MicrosoftVisualStudioThreadingVersion>
<MicrosoftVisualStudioThreadingVersion>17.3.1-alpha</MicrosoftVisualStudioThreadingVersion>
<MicrosoftVisualStudioUtilitiesVersion>$(VisualStudioContractPackagesVersion)</MicrosoftVisualStudioUtilitiesVersion>
<MicrosoftVisualStudioValidationVersion>17.0.46</MicrosoftVisualStudioValidationVersion>
<MicrosoftVisualStudioValidationVersion>17.0.53</MicrosoftVisualStudioValidationVersion>
<MicrosoftVisualStudioWCFReferenceInteropVersion>9.0.30729</MicrosoftVisualStudioWCFReferenceInteropVersion>
<SystemRuntimeCompilerServicesUnsafeVersion>6.0.0</SystemRuntimeCompilerServicesUnsafeVersion>
<VSSDKDebuggerVisualizersVersion>12.0.4</VSSDKDebuggerVisualizersVersion>
Expand Down Expand Up @@ -216,7 +217,7 @@
<NUnitLiteVersion>3.11.0</NUnitLiteVersion>
<NunitXmlTestLoggerVersion>2.1.80</NunitXmlTestLoggerVersion>
<RoslynToolsSignToolVersion>1.0.0-beta2-dev3</RoslynToolsSignToolVersion>
<StreamJsonRpcVersion>2.11.34</StreamJsonRpcVersion>
<StreamJsonRpcVersion>2.12.7-alpha</StreamJsonRpcVersion>
<NerdbankStreamsVersion>2.8.57</NerdbankStreamsVersion>
<XUnitVersion>2.4.1</XUnitVersion>
<XUnitRunnerVersion>2.4.2</XUnitRunnerVersion>
Expand Down
18 changes: 16 additions & 2 deletions src/Compiler/Checking/AttributeChecking.fs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ open FSharp.Compiler.TypeHierarchy
#if !NO_TYPEPROVIDERS
open FSharp.Compiler.TypeProviders
open FSharp.Core.CompilerServices
open Features

#endif

exception ObsoleteWarning of string * range
Expand Down Expand Up @@ -236,8 +238,20 @@ let private CheckILAttributes (g: TcGlobals) isByrefLikeTyconRef cattrs m =
| Some ([ILAttribElem.String (Some msg) ], _) when not isByrefLikeTyconRef ->
WarnD(ObsoleteWarning(msg, m))
| Some ([ILAttribElem.String (Some msg); ILAttribElem.Bool isError ], _) when not isByrefLikeTyconRef ->
if isError then
ErrorD (ObsoleteError(msg, m))
if isError then
if g.langVersion.SupportsFeature(LanguageFeature.RequiredPropertiesSupport) then
// In some cases C# will generate both ObsoleteAttribute and CompilerFeatureRequiredAttribute.
// Specifically, when default constructor is generated for class with any reqired members in them.
// ObsoleteAttribute should be ignored if CompilerFeatureRequiredAttribute is present, and its name is "RequiredMembers".
// REVIEW: Shall feature names be moved to LanguageFeatures (or elsewhere), and be tied to actual features somehow?
let (AttribInfo(tref,_)) = g.attrib_CompilerFeatureRequiredAttribute
match TryDecodeILAttribute tref cattrs with
| Some([ILAttribElem.String (Some featureName) ], _) when featureName = "RequiredMembers" ->
CompleteD
| _ ->
ErrorD (ObsoleteError(msg, m))
else
ErrorD (ObsoleteError(msg, m))
else
WarnD (ObsoleteWarning(msg, m))
| Some ([ILAttribElem.String None ], _) when not isByrefLikeTyconRef ->
Expand Down
57 changes: 55 additions & 2 deletions src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8964,6 +8964,20 @@ and TcLookupThen cenv overallTy env tpenv mObjExpr objExpr objExprTy longId dela
// To get better warnings we special case some of the few known mutate-a-struct method names
let mutates = (if methodName = "MoveNext" || methodName = "GetNextArg" then DefinitelyMutates else PossiblyMutates)

if g.langVersion.SupportsFeature(LanguageFeature.InitPropertiesSupport) then
let minfo = List.head minfos

// Check, wheter this method has `IsExternalInit`, emit an error diagnostic in this case.
let hasInitOnlyMod =
match minfo with
| ILMeth (_, ilMethInfo, _) ->
match ilMethInfo.ILMethodRef.ReturnType with
| ILType.Modified(_, cls, _) -> cls.FullName = "System.Runtime.CompilerServices.IsExternalInit"
| _ -> false
| _ -> false
if hasInitOnlyMod then
errorR (Error (FSComp.SR.tcGetterForInitOnlyPropertyCannotBeCalled1 methodName, mItem))

#if !NO_TYPEPROVIDERS
match TryTcMethodAppToStaticConstantArgs cenv env tpenv (minfos, tyArgsOpt, mExprAndItem, mItem) with
| Some minfoAfterStaticArguments ->
Expand Down Expand Up @@ -9010,6 +9024,10 @@ and TcLookupThen cenv overallTy env tpenv mObjExpr objExpr objExprTy longId dela
if isNil meths then error (Error (FSComp.SR.tcPropertyIsNotReadable nm, mItem))
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mExprAndItem mItem nm ad PossiblyMutates true meths afterResolution NormalValUse args atomicFlag delayed
else

if g.langVersion.SupportsFeature(LanguageFeature.RequiredPropertiesSupport) && pinfo.IsSetterInitOnly then
errorR (Error (FSComp.SR.tcInitOnlyPropertyCannotBeSet1 nm, mItem))

let args = if pinfo.IsIndexer then args else []
let mut = (if isStructTy g (tyOfExpr g objExpr) then DefinitelyMutates else PossiblyMutates)
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mStmt mItem nm ad mut true meths afterResolution NormalValUse (args @ [expr2]) atomicFlag []
Expand Down Expand Up @@ -9713,6 +9731,37 @@ and TcMethodApplication
// Handle post-hoc property assignments
let setterExprPrebinders, callExpr3 =
let expr = callExpr2b

// Make sure, if apparent type has any required properties, they all are in the `finalAssignedItemSetters`.
// If if is a constructor, and it is not marked with `SetsRequiredMembersAttributeAttribute`, then:
// 1. Get all properties of the type.
// 2. Check if any of them has `IsRequired` set.
// 2.1. If there are none, proceed as usual
// 2.2. If there are any, make sure all of them (or their setters) are in `finalAssignedItemSetters`.
// 3. If some are missing, produce a diagnostic which missing ones.
if g.langVersion.SupportsFeature(LanguageFeature.RequiredPropertiesSupport) && finalCalledMethInfo.IsConstructor then
let requiredProps =
match finalCalledMethInfo with
| ILMeth (_, ilmethInfo, _) ->
if TryFindILAttribute g.attrib_SetsRequiredMembersAttribute ilmethInfo.RawMetadata.CustomAttrs then
[]
else
let props = GetImmediateIntrinsicPropInfosOfType (None, AccessibleFromSomeFSharpCode) g cenv.amap range0 ilmethInfo.ApparentEnclosingType
List.filter (fun (p: PropInfo) -> p.IsRequired ) props
| _ -> []

if requiredProps.Length > 0 then
let setterPropNames =
finalAssignedItemSetters
|> List.choose (function | AssignedItemSetter(_, AssignedPropSetter (pinfo, _, _), _) -> Some pinfo.PropertyName | _ -> None)

let missingProps =
requiredProps
|> List.filter (fun pinfo -> not (List.contains pinfo.PropertyName setterPropNames))
if missingProps.Length > 0 then
let details = NicePrint.multiLineStringOfPropInfos g cenv.amap mMethExpr env.DisplayEnv missingProps
errorR(Error(FSComp.SR.tcMissingRequiredMembers details, mMethExpr))

if isCheckingAttributeCall then
[], expr
elif isNil finalAssignedItemSetters then
Expand All @@ -9724,7 +9773,7 @@ and TcMethodApplication
// Build the expression that mutates the properties on the result of the call
let setterExprPrebinders, propSetExpr =
(mkUnit g mMethExpr, finalAssignedItemSetters) ||> List.mapFold (fun acc assignedItemSetter ->
let argExprPrebinder, action, m = TcSetterArgExpr cenv env denv objExpr ad assignedItemSetter
let argExprPrebinder, action, m = TcSetterArgExpr cenv env denv objExpr ad assignedItemSetter finalCalledMethInfo.IsConstructor
argExprPrebinder, mkCompGenSequential m acc action)

// now put them together
Expand Down Expand Up @@ -9770,7 +9819,7 @@ and TcMethodApplication
(callExpr6, finalAttributeAssignedNamedItems, delayed), tpenv

/// For Method(X = expr) 'X' can be a property, IL Field or F# record field
and TcSetterArgExpr cenv env denv objExpr ad (AssignedItemSetter(id, setter, CallerArg(callerArgTy, m, isOptCallerArg, argExpr))) =
and TcSetterArgExpr cenv env denv objExpr ad (AssignedItemSetter(id, setter, CallerArg(callerArgTy, m, isOptCallerArg, argExpr))) calledFromConstructor =
let g = cenv.g

if isOptCallerArg then
Expand All @@ -9779,6 +9828,10 @@ and TcSetterArgExpr cenv env denv objExpr ad (AssignedItemSetter(id, setter, Cal
let argExprPrebinder, action, defnItem =
match setter with
| AssignedPropSetter (pinfo, pminfo, pminst) ->

if g.langVersion.SupportsFeature(LanguageFeature.RequiredPropertiesSupport) && pinfo.IsSetterInitOnly && not calledFromConstructor then
errorR (Error (FSComp.SR.tcInitOnlyPropertyCannotBeSet1 pinfo.PropertyName, m))

MethInfoChecks g cenv.amap true None [objExpr] ad m pminfo
let calledArgTy = List.head (List.head (pminfo.GetParamTypes(cenv.amap, m, pminst)))
let tcVal = LightweightTcValForUsingInBuildMethodCall g
Expand Down
14 changes: 14 additions & 0 deletions src/Compiler/Checking/NicePrint.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,10 @@ module InfoMemberPrinting =
layoutType denv retTy ^^
getterSetter

let formatPropInfoToBufferFreeStyle g amap m denv os (pinfo: PropInfo) =
let resL = prettyLayoutOfPropInfoFreeStyle g amap m denv pinfo
resL |> bufferL os

let formatMethInfoToBufferFreeStyle amap m denv os (minfo: MethInfo) =
let _, resL = prettyLayoutOfMethInfoFreeStyle amap m denv emptyTyparInst minfo
resL |> bufferL os
Expand Down Expand Up @@ -2461,6 +2465,16 @@ let multiLineStringOfMethInfos infoReader m denv minfos =
|> List.map (sprintf "%s %s" Environment.NewLine)
|> String.concat ""

let stringOfPropInfo g amap m denv pinfo =
buildString (fun buf -> InfoMemberPrinting.formatPropInfoToBufferFreeStyle g amap m denv buf pinfo)

/// Convert PropInfos to lines separated by newline including a newline as the first character
let multiLineStringOfPropInfos g amap m denv pinfos =
pinfos
|> List.map (stringOfPropInfo g amap m denv)
|> List.map (sprintf "%s %s" Environment.NewLine)
|> String.concat ""

/// Convert a ParamData to a string
let stringOfParamData denv paramData = buildString (fun buf -> InfoMemberPrinting.formatParamDataToBuffer denv buf paramData)

Expand Down
5 changes: 5 additions & 0 deletions src/Compiler/Checking/NicePrint.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ val stringOfMethInfo: infoReader: InfoReader -> m: range -> denv: DisplayEnv ->
val multiLineStringOfMethInfos:
infoReader: InfoReader -> m: range -> denv: DisplayEnv -> minfos: MethInfo list -> string

val stringOfPropInfo: g: TcGlobals -> amap: ImportMap -> m: range -> denv: DisplayEnv -> pinfo: PropInfo -> string

val multiLineStringOfPropInfos:
g: TcGlobals -> amap: ImportMap -> m: range -> denv: DisplayEnv -> pinfos: PropInfo list -> string

val stringOfParamData: denv: DisplayEnv -> paramData: ParamData -> string

val layoutOfParamData: denv: DisplayEnv -> paramData: ParamData -> Layout
Expand Down
24 changes: 24 additions & 0 deletions src/Compiler/Checking/infos.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1562,6 +1562,12 @@ type ILPropInfo =
/// Indicates if the IL property has a 'set' method
member x.HasSetter = Option.isSome x.RawMetadata.SetMethod

/// Indidcates whether IL property has an init-only setter (i.e. has the `System.Runtime.CompilerServices.IsExternalInit` modifer)
member x.IsSetterInitOnly =
match x.SetterMethod.ILMethodRef.ReturnType with
| ILType.Modified(_, cls, _) -> cls.FullName = "System.Runtime.CompilerServices.IsExternalInit"
| _ -> false

/// Indicates if the IL property is static
member x.IsStatic = (x.RawMetadata.CallingConv = ILThisConvention.Static)

Expand All @@ -1575,6 +1581,8 @@ type ILPropInfo =
(x.HasGetter && x.GetterMethod.IsNewSlot) ||
(x.HasSetter && x.SetterMethod.IsNewSlot)

member x.IsRequired = TryFindILAttribute x.TcGlobals.attrib_RequiredMemberAttribute x.RawMetadata.CustomAttrs

/// Get the names and types of the indexer arguments associated with the IL property.
///
/// Any type parameters of the enclosing type are instantiated in the type returned.
Expand Down Expand Up @@ -1688,6 +1696,22 @@ type PropInfo =
| ProvidedProp(_, pi, m) -> pi.PUntaint((fun pi -> pi.CanWrite), m)
#endif

member x.IsSetterInitOnly =
match x with
| ILProp ilpinfo -> ilpinfo.IsSetterInitOnly
| FSProp _ -> false
#if !NO_TYPEPROVIDERS
| ProvidedProp _ -> false
#endif

member x.IsRequired =
match x with
| ILProp ilpinfo -> ilpinfo.IsRequired
| FSProp _ -> false
#if !NO_TYPEPROVIDERS
| ProvidedProp _ -> false
#endif

/// Indicates if this is an extension member
member x.IsExtensionMember =
match x.ArbitraryValRef with
Expand Down
9 changes: 9 additions & 0 deletions src/Compiler/Checking/infos.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,9 @@ type ILPropInfo =
/// Get the declaring IL type of the IL property, including any generic instantiation
member ILTypeInfo: ILTypeInfo

/// Is the property requied (has the RequiredMemberAttribute).
member IsRequired: bool

/// Indicates if the IL property is logically a 'newslot', i.e. hides any previous slots of the same name.
member IsNewSlot: bool

Expand Down Expand Up @@ -787,6 +790,12 @@ type PropInfo =
/// Indicates if this property has an associated setter method.
member HasSetter: bool

/// Indidcates whether IL property has an init-only setter (i.e. has the `System.Runtime.CompilerServices.IsExternalInit` modifer)
member IsSetterInitOnly: bool

/// Is the property requied (has the RequiredMemberAttribute).
member IsRequired: bool

member ImplementedSlotSignatures: SlotSig list

/// Indicates if this property is marked 'override' and thus definitely overrides another property.
Expand Down
Loading