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

Obsolete attribute is ignored in constructor property assignment. #16900

Merged
merged 24 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
925f033
Obsolete attribute is ignored in constructor property assignment
edgarfgp Mar 11, 2024
a5f8032
Raise an warning when Obsolete attribute is used in constructor prope…
edgarfgp Mar 11, 2024
4558fff
release notes
edgarfgp Mar 21, 2024
35997f9
Merge branch 'main' into fix-11868
edgarfgp Mar 21, 2024
ed89056
Merge branch 'main' into fix-11868
edgarfgp Mar 22, 2024
1ae43a5
Merge branch 'main' into fix-11868
edgarfgp Mar 25, 2024
aa1c61e
Use a more accuarate range
edgarfgp Mar 25, 2024
9f51d0e
Merge branch 'fix-11868' of https://github.com/edgarfgp/fsharp into f…
edgarfgp Mar 25, 2024
93a0f1a
GetImmediateIntrinsicPropInfosOfType
edgarfgp Mar 25, 2024
ea5e736
more tests
edgarfgp Mar 25, 2024
b831f7f
Merge branch 'main' into fix-11868
edgarfgp Mar 25, 2024
7655d24
more tests
edgarfgp Mar 26, 2024
f33f922
Update logic to check one case at the time
edgarfgp Mar 26, 2024
1aa1188
Merge branch 'fix-11868' of https://github.com/edgarfgp/fsharp into f…
edgarfgp Mar 26, 2024
809ea70
getConstructorArgs
edgarfgp Mar 26, 2024
201470a
FIx PR comments
edgarfgp Mar 26, 2024
c63aa76
Merge branch 'main' into fix-11868
edgarfgp Mar 26, 2024
2d24709
Merge branch 'main' into fix-11868
edgarfgp Mar 26, 2024
a112243
Merge branch 'main' into fix-11868
edgarfgp Mar 27, 2024
af745e0
Simplify check
edgarfgp Mar 28, 2024
299a188
even more simpler
edgarfgp Mar 28, 2024
ffadcf8
Merge branch 'main' into fix-11868
edgarfgp Mar 29, 2024
a462bd9
Merge branch 'main' into fix-11868
edgarfgp Apr 2, 2024
ed1196c
Merge branch 'main' into fix-11868
edgarfgp Apr 2, 2024
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
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* Enforce AttributeTargets on enums ([PR #16887](https://github.com/dotnet/fsharp/pull/16887))
* Completion: fix for unfinished record field decl ([PR #16893](https://github.com/dotnet/fsharp/pull/16893))
* Enforce AttributeTargets on delegates ([PR #16891](https://github.com/dotnet/fsharp/pull/16891))
* Obsolete attribute is ignored in constructor property assignment ([PR #16900](https://github.com/dotnet/fsharp/pull/16900))
* Completion: fix completion in empty dot lambda prefix ([#16829](https://github.com/dotnet/fsharp/pull/16829))
* Fix StackOverflow when checking non-recursive bindings in module or namespace in `fscAnyCpu`/`fsiAnyCpu`. ([PR #16908](https://github.com/dotnet/fsharp/pull/16908))

Expand Down
78 changes: 59 additions & 19 deletions src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1234,12 +1234,34 @@ let CheckForAbnormalOperatorNames (cenv: cenv) (idRange: range) coreDisplayName
if isMember then
warning(StandardOperatorRedefinitionWarning(FSComp.SR.tcInvalidMemberNameFixedTypes opName, idRange))
| Other -> ()

let GetConstructorArgsIdents synExprs =
let rec getConstructorArgs expr =
match expr with
| SynExpr.Paren(expr = SynExpr.App(funcExpr = expr)) -> getConstructorArgs expr
| SynExpr.Paren(expr = SynExpr.Tuple(exprs = synExprs)) -> synExprs |> List.collect getConstructorArgs
| SynExpr.App(funcExpr = expr1; argExpr = expr2) -> [ expr1 ; expr2 ] |> List.collect getConstructorArgs
| SynExpr.Ident(id) -> [ id ]
| _ -> []

synExprs |> List.collect getConstructorArgs


let CheckInitProperties (g: TcGlobals) (minfo: MethInfo) methodName mItem =
if g.langVersion.SupportsFeature(LanguageFeature.InitPropertiesSupport) then
// Check, wheter this method has external init, emit an error diagnostic in this case.
if minfo.HasExternalInit then
errorR (Error (FSComp.SR.tcSetterForInitOnlyPropertyCannotBeCalled1 methodName, mItem))

let CheckPropertyAttributes finalAssignedItemSetters (ctorArgs: Ident list) =
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
let propInfos =
finalAssignedItemSetters
|> List.choose (function | AssignedItemSetter(_, AssignedPropSetter (_, pinfo, _, _), _) -> Some pinfo | _ -> None)

for propInfo in propInfos do
for arg in ctorArgs do
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
if arg.idText = propInfo.DisplayName then
CheckPropInfoAttributes propInfo arg.idRange |> CommitOperationResult

let CheckRequiredProperties (g:TcGlobals) (env: TcEnv) (cenv: TcFileState) (minfo: MethInfo) finalAssignedItemSetters mMethExpr =
// Make sure, if apparent type has any required properties, they all are in the `finalAssignedItemSetters`.
Expand Down Expand Up @@ -6651,11 +6673,25 @@ and TcCtorCall isNaked cenv env tpenv (overallTy: OverallTy) objTy mObjTyOpt ite
error(Error((if superInit then FSComp.SR.tcInheritCannotBeUsedOnInterfaceType() else FSComp.SR.tcNewCannotBeUsedOnInterfaceType()), mWholeCall))

match item, args with
| Item.CtorGroup(methodName, minfos), _ ->
| Item.CtorGroup(methodName, minfos), argExprs ->
let meths = List.map (fun minfo -> minfo, None) minfos
if isNaked && TypeFeasiblySubsumesType 0 g cenv.amap mWholeCall g.system_IDisposable_ty NoCoerce objTy then
warning(Error(FSComp.SR.tcIDisposableTypeShouldUseNew(), mWholeCall))

let ctorArgs = GetConstructorArgsIdents argExprs

if not ctorArgs.IsEmpty then
// Here we will have the ValRefs(Members, Properties) from F# types
let valRefs =
minfos
|> List.collect (fun minfo-> minfo.ApparentEnclosingTyconRef.MembersOfFSharpTyconSorted)
edgarfgp marked this conversation as resolved.
Show resolved Hide resolved
|> List.filter (fun v -> CheckFSharpAttributesForObsolete g v.Attribs)

for vref in valRefs do
for arg in ctorArgs do
if arg.idText = vref.DisplayName then
CheckValAttributes g vref arg.idRange |> CommitOperationResult

// Check the type is not abstract
// skip this check if this ctor call is either 'inherit(...)' or call is located within constructor shape
if not (superInit || AreWithinCtorShape env)
Expand All @@ -6667,7 +6703,7 @@ and TcCtorCall isNaked cenv env tpenv (overallTy: OverallTy) objTy mObjTyOpt ite
| Some mObjTy, None -> ForNewConstructors cenv.tcSink env mObjTy methodName minfos
| None, _ -> AfterResolution.DoNothing

TcMethodApplicationThen cenv env overallTy (Some objTy) tpenv None [] mWholeCall mItem methodName ad PossiblyMutates false meths afterResolution isSuperInit args ExprAtomicFlag.NonAtomic None delayed
TcMethodApplicationThen cenv env overallTy (Some objTy) tpenv None [] mWholeCall mItem methodName ad PossiblyMutates false meths afterResolution isSuperInit args ExprAtomicFlag.NonAtomic None delayed ctorArgs

| Item.DelegateCtor ty, [arg] ->
// Re-record the name resolution since we now know it's a constructor call
Expand Down Expand Up @@ -7082,7 +7118,7 @@ and TcObjectExpr (cenv: cenv) env tpenv (objTy, realObjTy, argopt, binds, extraI
let afterResolution = ForNewConstructors cenv.tcSink env mObjTy methodName minfos
let ad = env.AccessRights

let expr, tpenv = TcMethodApplicationThen cenv env (MustEqual objTy) None tpenv None [] mWholeExpr mObjTy methodName ad PossiblyMutates false meths afterResolution CtorValUsedAsSuperInit [arg] ExprAtomicFlag.Atomic None []
let expr, tpenv = TcMethodApplicationThen cenv env (MustEqual objTy) None tpenv None [] mWholeExpr mObjTy methodName ad PossiblyMutates false meths afterResolution CtorValUsedAsSuperInit [arg] ExprAtomicFlag.Atomic None [] []
// The 'base' value is always bound
let baseIdOpt = (match baseIdOpt with None -> Some(ident("base", mObjTy)) | Some id -> Some id)
expr, baseIdOpt, tpenv
Expand Down Expand Up @@ -8723,7 +8759,7 @@ and TcMethodItemThen (cenv: cenv) overallTy env item methodName minfos tpenv mIt
let meths = List.map (fun minfo -> minfo, None) minfos
match delayed with
| DelayedApp (atomicFlag, _, _, arg, mExprAndArg) :: otherDelayed ->
TcMethodApplicationThen cenv env overallTy None tpenv None [] mExprAndArg mItem methodName ad NeverMutates false meths afterResolution NormalValUse [arg] atomicFlag staticTyOpt otherDelayed
TcMethodApplicationThen cenv env overallTy None tpenv None [] mExprAndArg mItem methodName ad NeverMutates false meths afterResolution NormalValUse [arg] atomicFlag staticTyOpt otherDelayed []

| DelayedTypeApp(tys, mTypeArgs, mExprAndTypeArgs) :: otherDelayed ->

Expand All @@ -8737,9 +8773,9 @@ and TcMethodItemThen (cenv: cenv) overallTy env item methodName minfos tpenv mIt

match otherDelayed with
| DelayedApp(atomicFlag, _, _, arg, mExprAndArg) :: otherDelayed ->
TcMethodApplicationThen cenv env overallTy None tpenv None [] mExprAndArg mItem methodName ad NeverMutates false [(minfoAfterStaticArguments, None)] afterResolution NormalValUse [arg] atomicFlag staticTyOpt otherDelayed
TcMethodApplicationThen cenv env overallTy None tpenv None [] mExprAndArg mItem methodName ad NeverMutates false [(minfoAfterStaticArguments, None)] afterResolution NormalValUse [arg] atomicFlag staticTyOpt otherDelayed []
| _ ->
TcMethodApplicationThen cenv env overallTy None tpenv None [] mExprAndTypeArgs mItem methodName ad NeverMutates false [(minfoAfterStaticArguments, None)] afterResolution NormalValUse [] ExprAtomicFlag.Atomic staticTyOpt otherDelayed
TcMethodApplicationThen cenv env overallTy None tpenv None [] mExprAndTypeArgs mItem methodName ad NeverMutates false [(minfoAfterStaticArguments, None)] afterResolution NormalValUse [] ExprAtomicFlag.Atomic staticTyOpt otherDelayed []

| None ->
#endif
Expand All @@ -8753,16 +8789,16 @@ and TcMethodItemThen (cenv: cenv) overallTy env item methodName minfos tpenv mIt

match otherDelayed with
| DelayedApp(atomicFlag, _, _, arg, mExprAndArg) :: otherDelayed ->
TcMethodApplicationThen cenv env overallTy None tpenv (Some tyargs) [] mExprAndArg mItem methodName ad NeverMutates false meths afterResolution NormalValUse [arg] atomicFlag staticTyOpt otherDelayed
TcMethodApplicationThen cenv env overallTy None tpenv (Some tyargs) [] mExprAndArg mItem methodName ad NeverMutates false meths afterResolution NormalValUse [arg] atomicFlag staticTyOpt otherDelayed []
| _ ->
TcMethodApplicationThen cenv env overallTy None tpenv (Some tyargs) [] mExprAndTypeArgs mItem methodName ad NeverMutates false meths afterResolution NormalValUse [] ExprAtomicFlag.Atomic staticTyOpt otherDelayed
TcMethodApplicationThen cenv env overallTy None tpenv (Some tyargs) [] mExprAndTypeArgs mItem methodName ad NeverMutates false meths afterResolution NormalValUse [] ExprAtomicFlag.Atomic staticTyOpt otherDelayed []

| _ ->
#if !NO_TYPEPROVIDERS
if not minfos.IsEmpty && minfos[0].ProvidedStaticParameterInfo.IsSome then
error(Error(FSComp.SR.etMissingStaticArgumentsToMethod(), mItem))
#endif
TcMethodApplicationThen cenv env overallTy None tpenv None [] mItem mItem methodName ad NeverMutates false meths afterResolution NormalValUse [] ExprAtomicFlag.Atomic staticTyOpt delayed
TcMethodApplicationThen cenv env overallTy None tpenv None [] mItem mItem methodName ad NeverMutates false meths afterResolution NormalValUse [] ExprAtomicFlag.Atomic staticTyOpt delayed []

and TcCtorItemThen (cenv: cenv) overallTy env item nm minfos tinstEnclosing tpenv mItem afterResolution delayed =
#if !NO_TYPEPROVIDERS
Expand Down Expand Up @@ -9130,19 +9166,19 @@ and TcPropertyItemThen cenv overallTy env nm pinfos tpenv mItem afterResolution

// x.P <- ... byref setter
if isNil meths then error (Error (FSComp.SR.tcPropertyIsNotReadable nm, mItem))
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt [] mItem mItem nm ad NeverMutates true meths afterResolution NormalValUse args ExprAtomicFlag.Atomic staticTyOpt delayed
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt [] mItem mItem nm ad NeverMutates true meths afterResolution NormalValUse args ExprAtomicFlag.Atomic staticTyOpt delayed []
else
let args = if pinfo.IsIndexer then args else []
if isNil meths then
errorR (Error (FSComp.SR.tcPropertyCannotBeSet1 nm, mItem))
// Note: static calls never mutate a struct object argument
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt [] mStmt mItem nm ad NeverMutates true meths afterResolution NormalValUse (args@[expr2]) ExprAtomicFlag.NonAtomic staticTyOpt otherDelayed
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt [] mStmt mItem nm ad NeverMutates true meths afterResolution NormalValUse (args@[expr2]) ExprAtomicFlag.NonAtomic staticTyOpt otherDelayed []
| _ ->
// Static Property Get (possibly indexer)
let meths = pinfos |> GettersOfPropInfos
if isNil meths then error (Error (FSComp.SR.tcPropertyIsNotReadable nm, mItem))
// Note: static calls never mutate a struct object argument
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt [] mItem mItem nm ad NeverMutates true meths afterResolution NormalValUse args ExprAtomicFlag.Atomic staticTyOpt delayed
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt [] mItem mItem nm ad NeverMutates true meths afterResolution NormalValUse args ExprAtomicFlag.Atomic staticTyOpt delayed []

and TcILFieldItemThen cenv overallTy env finfo tpenv mItem delayed =
let g = cenv.g
Expand Down Expand Up @@ -9294,7 +9330,7 @@ and TcLookupItemThen cenv overallTy env tpenv mObjExpr objExpr objExprTy delayed
let item = Item.MethodGroup(methodName, [minfoAfterStaticArguments], Some minfos[0])
CallNameResolutionSinkReplacing cenv.tcSink (mExprAndItem, env.NameEnv, item, [], ItemOccurence.Use, env.eAccessRights)

TcMethodApplicationThen cenv env overallTy None tpenv None objArgs mExprAndItem mItem methodName ad mutates false [(minfoAfterStaticArguments, None)] afterResolution NormalValUse args atomicFlag None delayed
TcMethodApplicationThen cenv env overallTy None tpenv None objArgs mExprAndItem mItem methodName ad mutates false [(minfoAfterStaticArguments, None)] afterResolution NormalValUse args atomicFlag None delayed []
| None ->
if not minfos.IsEmpty && minfos[0].ProvidedStaticParameterInfo.IsSome then
error(Error(FSComp.SR.etMissingStaticArgumentsToMethod(), mItem))
Expand All @@ -9303,7 +9339,7 @@ and TcLookupItemThen cenv overallTy env tpenv mObjExpr objExpr objExprTy delayed
let tyArgsOpt, tpenv = TcMemberTyArgsOpt cenv env tpenv tyArgsOpt
let meths = minfos |> List.map (fun minfo -> minfo, None)

TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mExprAndItem mItem methodName ad mutates false meths afterResolution NormalValUse args atomicFlag None delayed
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mExprAndItem mItem methodName ad mutates false meths afterResolution NormalValUse args atomicFlag None delayed []

| Item.Property (nm, pinfos, _) ->
// Instance property
Expand Down Expand Up @@ -9331,20 +9367,20 @@ and TcLookupItemThen cenv overallTy env tpenv mObjExpr objExpr objExprTy delayed
errorR (Error (FSComp.SR.tcPropertyCannotBeSet1 nm, mItem))
// x.P <- ... byref setter
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 None delayed
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mExprAndItem mItem nm ad PossiblyMutates true meths afterResolution NormalValUse args atomicFlag None 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 None []
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mStmt mItem nm ad mut true meths afterResolution NormalValUse (args @ [expr2]) atomicFlag None [] []
| _ ->
// Instance property getter
let meths = GettersOfPropInfos pinfos
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 None delayed
TcMethodApplicationThen cenv env overallTy None tpenv tyArgsOpt objArgs mExprAndItem mItem nm ad PossiblyMutates true meths afterResolution NormalValUse args atomicFlag None delayed []

| Item.RecdField rfinfo ->
// Get or set instance F# field or literal
Expand Down Expand Up @@ -9507,6 +9543,7 @@ and TcMethodApplicationThen
atomicFlag // is the expression atomic or not?
staticTyOpt // is there a static type that governs the call, different to the nominal type containing the member, e.g. 'T.CallSomeMethod()
delayed // further lookups and applications that follow this
ctorArgs // arguments to the constructor, if this is a constructor call
=

let g = cenv.g
Expand All @@ -9523,7 +9560,7 @@ and TcMethodApplicationThen

// Call the helper below to do the real checking
let (expr, attributeAssignedNamedItems, delayed), tpenv =
TcMethodApplication false cenv env tpenv callerTyArgs objArgs mWholeExpr mItem methodName objTyOpt ad mut isProp meths afterResolution isSuperInit args exprTy staticTyOpt delayed
TcMethodApplication false cenv env tpenv callerTyArgs objArgs mWholeExpr mItem methodName objTyOpt ad mut isProp meths afterResolution isSuperInit args exprTy staticTyOpt delayed ctorArgs

// Give errors if some things couldn't be assigned
if not (isNil attributeAssignedNamedItems) then
Expand Down Expand Up @@ -9894,6 +9931,7 @@ and TcMethodApplication
(exprTy: OverallTy)
staticTyOpt // is there a static type that governs the call, different to the nominal type containing the member, e.g. 'T.CallSomeMethod()
delayed
ctorArgs
=

let g = cenv.g
Expand Down Expand Up @@ -10086,6 +10124,8 @@ and TcMethodApplication
// Handle post-hoc property assignments
let setterExprPrebinders, callExpr2b =
let expr = callExpr2

CheckPropertyAttributes finalAssignedItemSetters ctorArgs

CheckRequiredProperties g env cenv finalCalledMethInfo finalAssignedItemSetters mMethExpr

Expand Down Expand Up @@ -11072,7 +11112,7 @@ and TcAttributeEx canFail (cenv: cenv) (env: TcEnv) attrTgt attrEx (synAttr: Syn
let meths = minfos |> List.map (fun minfo -> minfo, None)
let afterResolution = ForNewConstructors cenv.tcSink env tyid.idRange methodName minfos
let (expr, attributeAssignedNamedItems, _), _ =
TcMethodApplication true cenv env tpenv None [] mAttr mAttr methodName None ad PossiblyMutates false meths afterResolution NormalValUse [arg] (MustEqual ty) None []
TcMethodApplication true cenv env tpenv None [] mAttr mAttr methodName None ad PossiblyMutates false meths afterResolution NormalValUse [arg] (MustEqual ty) None [] []

UnifyTypes cenv env mAttr ty (tyOfExpr g expr)

Expand Down
Loading
Loading