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

Bugfix :: Nullness :: Add warnings for instantiations of (T|null) which are not allowed in F# (frequent at Deserialize from S.T.Json) #18057

Merged
15 changes: 9 additions & 6 deletions FSharp.sln
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "TestTP", "tests\service\dat
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Docs", "Docs", "{4E4F41D9-86A7-4F5D-B735-1A0744AB68AC}"
ProjectSection(SolutionItems) = preProject
docs\builder-caches.md = docs\builder-caches.md
docs\changing-the-ast.md = docs\changing-the-ast.md
docs\coding-standards.md = docs\coding-standards.md
docs\compiler-startup-performance.md = docs\compiler-startup-performance.md
docs\debug-emit.md = docs\debug-emit.md
Expand All @@ -57,12 +59,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Docs", "Docs", "{4E4F41D9-8
docs\index.md = docs\index.md
docs\large-inputs-and-stack-overflows.md = docs\large-inputs-and-stack-overflows.md
docs\memory-usage.md = docs\memory-usage.md
docs\optimizations.md = docs\optimizations.md
docs\names.md = docs\names.md
docs\optimizations-equality.md = docs\optimizations-equality.md
docs\optimizations.md = docs\optimizations.md
docs\overview.md = docs\overview.md
docs\builder-caches.md = docs\builder-caches.md
docs\changing-the-ast.md = docs\changing-the-ast.md
docs\names.md = docs\names.md
docs\perf-discussions-archive.md = docs\perf-discussions-archive.md
docs\project-builds.md = docs\project-builds.md
docs\representations.md = docs\representations.md
Expand All @@ -84,8 +84,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "fcs", "fcs", "{B86EBFF1-E03
docs\fcs\symbols.fsx = docs\fcs\symbols.fsx
docs\fcs\tokenizer.fsx = docs\fcs\tokenizer.fsx
docs\fcs\typedtree.fsx = docs\fcs\typedtree.fsx
docs\fcs\untypedtree.fsx = docs\fcs\untypedtree.fsx
docs\fcs\untypedtree-apis.fsx = docs\fcs\untypedtree-apis.fsx
docs\fcs\untypedtree.fsx = docs\fcs\untypedtree.fsx
EndProjectSection
EndProject
Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "fsc", "src\fsc\fscProject\fsc.fsproj", "{10D15DBB-EFF0-428C-BA83-41600A93EEC4}"
Expand Down Expand Up @@ -142,6 +142,9 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".FSharp.Compiler.Service",
docs\release-notes\.FSharp.Compiler.Service\8.0.200.md = docs\release-notes\.FSharp.Compiler.Service\8.0.200.md
docs\release-notes\.FSharp.Compiler.Service\8.0.202.md = docs\release-notes\.FSharp.Compiler.Service\8.0.202.md
docs\release-notes\.FSharp.Compiler.Service\8.0.300.md = docs\release-notes\.FSharp.Compiler.Service\8.0.300.md
docs\release-notes\.FSharp.Compiler.Service\8.0.400.md = docs\release-notes\.FSharp.Compiler.Service\8.0.400.md
docs\release-notes\.FSharp.Compiler.Service\9.0.100.md = docs\release-notes\.FSharp.Compiler.Service\9.0.100.md
docs\release-notes\.FSharp.Compiler.Service\9.0.200.md = docs\release-notes\.FSharp.Compiler.Service\9.0.200.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".FSharp.Core", ".FSharp.Core", "{23798638-A1E9-4DAE-9C9C-F5D87499ADD6}"
Expand All @@ -158,8 +161,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".Language", ".Language", "{
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".VisualStudio", ".VisualStudio", "{AF70EC5A-8E7C-4FDA-857D-AF08082CFC64}"
ProjectSection(SolutionItems) = preProject
docs\release-notes\.VisualStudio\17.9.md = docs\release-notes\.VisualStudio\17.9.md
docs\release-notes\.VisualStudio\17.10.md = docs\release-notes\.VisualStudio\17.10.md
docs\release-notes\.VisualStudio\17.9.md = docs\release-notes\.VisualStudio\17.9.md
EndProjectSection
EndProject
Global
Expand Down
7 changes: 4 additions & 3 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.200.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@
* Fix internal error when analyzing incomplete inherit member ([PR #17905](https://github.com/dotnet/fsharp/pull/17905))
* Add warning when downcasting from nullable type to non-nullable ([PR #17965](https://github.com/dotnet/fsharp/pull/17965))
* Fix missing nullness warning in case of method resolution multiple candidates ([PR #17917](https://github.com/dotnet/fsharp/pull/17918))
* Fix failure to use bound values in `when` clauses of `try-with` in `seq` expressions ([# 17990](https://github.com/dotnet/fsharp/pull/17990))
* Fix failure to use bound values in `when` clauses of `try-with` in `seq` expressions ([PR #17990](https://github.com/dotnet/fsharp/pull/17990))

### Added

* Let `dotnet fsi --help` print a link to the documentation website. ([PR #18006](https://github.com/dotnet/fsharp/pull/18006))
* Deprecate places where `seq` can be omitted. ([Language suggestion #1033](https://github.com/fsharp/fslang-suggestions/issues/1033), [PR #17772](https://github.com/dotnet/fsharp/pull/17772))
* Support literal attribute on decimals ([PR #17769](https://github.com/dotnet/fsharp/pull/17769))
* Added type conversions cache, only enabled for compiler runs, guarded by language version preview ([PR#17668](https://github.com/dotnet/fsharp/pull/17668))
* Added project property ParallelCompilation which turns on graph based type checking, parallel ILXGen and parallel optimization. By default on for users of langversion=preview ([PR#17948](https://github.com/dotnet/fsharp/pull/17948))
* Added type conversions cache, only enabled for compiler runs, guarded by language version preview ([PR #17668](https://github.com/dotnet/fsharp/pull/17668))
* Added project property ParallelCompilation which turns on graph based type checking, parallel ILXGen and parallel optimization. By default on for users of langversion=preview ([PR #17948](https://github.com/dotnet/fsharp/pull/17948))
* Adding warning when consuming generic method returning T|null for types not supporting nullness (structs,anons,tuples) ([PR #18057](https://github.com/dotnet/fsharp/pull/18057))

### Changed

Expand Down
5 changes: 4 additions & 1 deletion src/Compiler/Checking/Expressions/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9728,10 +9728,13 @@ and TcMethodApplicationThen
let (CallerNamedArg(id, _)) = List.head attributeAssignedNamedItems
errorR(Error(FSComp.SR.tcNamedArgumentDidNotMatch(id.idText), id.idRange))


// Resolve the "delayed" lookups
let exprTy = (tyOfExpr g expr)

for problematicTy in GetDisallowedNullness g exprTy do
let denv = env.DisplayEnv
warning(Error(FSComp.SR.tcDisallowedNullableApplication(methodName,NicePrint.minimalStringOfType denv problematicTy), m))

PropagateThenTcDelayed cenv overallTy env tpenv mWholeExpr (MakeApplicableExprNoFlex cenv expr) exprTy atomicFlag delayed

/// Infer initial type information at the callsite from the syntax of an argument, prior to overload resolution.
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/FSComp.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1538,6 +1538,7 @@ tcPassingWithoutNullToNonNullQuickAP,"You can remove this |NonNullQuick| pattern
tcPassingWithoutNullTononNullFunction,"You can remove this `nonNull` assertion."
3263,tcNullableToStringOverride,"With nullness checking enabled, overrides of .ToString() method must return a non-nullable string. You can handle potential nulls via the built-in string function."
3264,tcDowncastFromNullableToWithoutNull,"Nullness warning: Downcasting from '%s' into '%s' can introduce unexpected null values. Cast to '%s|null' instead or handle the null before downcasting."
3265,tcDisallowedNullableApplication,"Application of method '%s' attempted to create a nullable type ('T | null) for '%s'. Nullness warnings won't be reported correctly for such types."
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
3268,csNullNotNullConstraintInconsistent,"The constraints 'null' and 'not null' are inconsistent"
3271,tcNullnessCheckingNotEnabled,"The 'nullness checking' language feature is not enabled. This use of a nullness checking construct will be ignored."
csTypeHasNullAsTrueValue,"The type '%s' uses 'null' as a representation value but a non-null type is expected"
Expand Down
51 changes: 51 additions & 0 deletions src/Compiler/TypedTree/TypedTreeOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9215,6 +9215,57 @@ let reqTyForArgumentNullnessInference g actualTy reqTy =
changeWithNullReqTyToVariable g reqTy
| _ -> reqTy


let GetDisallowedNullness (g:TcGlobals) (ty:TType) =
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
if g.checkNullness then
let rec hasWithNullAnyWhere ty alreadyWrappedInOuterWithNull =
match ty with
| TType_var (tp, n) ->
let withNull = alreadyWrappedInOuterWithNull || n.TryEvaluate() = (ValueSome NullnessInfo.WithNull)
match tp.Solution with
| None -> []
| Some t -> hasWithNullAnyWhere t withNull

| TType_app (tcr, tinst, nullnessOrig) ->
let tyArgs = tinst |> List.collect (fun t -> hasWithNullAnyWhere t false)

match alreadyWrappedInOuterWithNull, tcr.TypeAbbrev with
| true, _ when isStructTyconRef tcr -> ty :: tyArgs
| true, _ when tcr.IsMeasureableReprTycon ->
match tcr.TypeReprInfo with
| TMeasureableRepr realType ->
if hasWithNullAnyWhere realType true |> List.isEmpty then
[]
else [ty]
| _ -> []
| true, Some tAbbrev -> (hasWithNullAnyWhere tAbbrev true) @ tyArgs
| _ -> tyArgs

| TType_tuple (_,tupTypes) ->
let inner = tupTypes |> List.collect (fun t -> hasWithNullAnyWhere t false)
if alreadyWrappedInOuterWithNull then ty :: inner else inner

| TType_anon (anon,tys) ->
let inner = tys |> List.collect (fun t -> hasWithNullAnyWhere t false)
if alreadyWrappedInOuterWithNull then ty :: inner else inner
| TType_fun (d, r, _) ->
(hasWithNullAnyWhere d false) @ (hasWithNullAnyWhere r false)

| TType_forall _ -> []
| TType_ucase _ -> []
| TType_measure m ->
if alreadyWrappedInOuterWithNull then
let measuresInside =
ListMeasureVarOccs m
|> List.choose (fun x -> x.Solution)
|> List.collect (fun x -> hasWithNullAnyWhere x true)
ty :: measuresInside
else []

hasWithNullAnyWhere ty false
else
[]

let TypeHasAllowNull (tcref:TyconRef) g m =
not tcref.IsStructOrEnumTycon &&
not (isByrefLikeTyconRef g m tcref) &&
Expand Down
5 changes: 5 additions & 0 deletions src/Compiler/TypedTree/TypedTreeOps.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -1815,6 +1815,11 @@ val TypeNullIsTrueValue: TcGlobals -> TType -> bool

val TypeNullIsExtraValue: TcGlobals -> range -> TType -> bool

/// A type coming via interop from C# can be holding a nullness combination not supported in F#.
/// Prime example are APIs marked as T|null applied to structs, tuples and anons.
/// Unsupported values can also be nested within generic type arguments, e.g. a List<Tuple<string,T|null>> applied to an anon.
val GetDisallowedNullness: TcGlobals -> TType -> TType list

val TypeHasAllowNull: TyconRef -> TcGlobals -> range -> bool

val TypeNullIsExtraValueNew: TcGlobals -> range -> TType -> bool
Expand Down
5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading