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

Warn when obj is inferred #13298

Merged
merged 50 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
4a24440
First pass at a warning when obj is inferred
Smaug123 Jun 14, 2022
ef5090f
Add more tests, some of which are failing
Smaug123 Jun 14, 2022
9e1c8c2
A couple more tests
Smaug123 Jun 14, 2022
4f0390c
Add test for simpler quotation case
Smaug123 Jun 15, 2022
4d48759
Turn diagnostic off by default
Smaug123 Jun 15, 2022
5794221
Add a test which is failing, no idea why
Smaug123 Jun 15, 2022
c09ac18
Update text
Smaug123 Jul 14, 2022
55fd658
Fix up messages
Smaug123 Jul 29, 2022
2200ddd
Merge main
Smaug123 Jul 29, 2022
b15bd8b
Restructure tests
Smaug123 Jul 29, 2022
97ffb68
Speculative commit to restore range information where required
Smaug123 Jul 29, 2022
ee81996
Another failing test
Smaug123 Jan 22, 2023
e8d64fa
Resolve merge
Smaug123 Jan 23, 2023
2235295
Merge main, sloppily
Smaug123 Jan 23, 2023
aa155b0
Merge main
Smaug123 Jan 23, 2023
030a5cb
Tidy
Smaug123 Jan 23, 2023
76ea258
Take 2
Smaug123 Jan 23, 2023
849a97f
Add another test
Smaug123 Jan 23, 2023
5bfef32
Merge main
Smaug123 Feb 1, 2023
25f3ba1
Add prospective test
Smaug123 Feb 3, 2023
248b7a5
Merge branch 'obj-inference-warning' into obj-inference-take-2
Smaug123 Apr 12, 2023
c47dce2
Merge branch 'main' into obj-inference-warning
Smaug123 Apr 12, 2023
c6242c6
Merge remote-tracking branch 'origin/main' into obj-inference-warning
Smaug123 May 30, 2023
286fe21
Fix build
Smaug123 May 31, 2023
49db33f
Adjust message
Smaug123 May 31, 2023
52dc05d
Add extra fallback typar ranges
Smaug123 Jun 2, 2023
88b7780
WIP extra tests
Smaug123 Jun 2, 2023
8485188
Add fallback range
Smaug123 Jun 2, 2023
609fe05
Revert
Smaug123 Jun 2, 2023
407040b
Reinstate
Smaug123 Jun 2, 2023
d6479c5
Merge branch 'add-fallback-range' into obj-inference-warning
Smaug123 Jun 2, 2023
a2e7f1e
More tests
Smaug123 Jun 2, 2023
adea31c
Merge branch 'main' into obj-inference-warning
Smaug123 Jun 2, 2023
5c991f9
Merge main
Smaug123 Jun 5, 2023
3ee4928
Downgrade warning
Smaug123 Jun 5, 2023
79e4de5
Merge branch 'obj-inference-warning' of github.com:Smaug123/fsharp-1 …
Smaug123 Jun 5, 2023
3356912
Fix tests after merge
Smaug123 Jun 5, 2023
d99753b
Merge branch 'main' into obj-inference-warning
Smaug123 Jun 6, 2023
862c87d
Gate the diagnostic
Smaug123 Jun 6, 2023
2a7d7d9
Fix error
Smaug123 Jun 8, 2023
5f4288d
Merge branch 'main' into obj-inference-warning
Smaug123 Jun 8, 2023
710bab0
Enable language version in tests
Smaug123 Jun 8, 2023
23601d5
Merge branch 'main' into obj-inference-warning
Smaug123 Jun 8, 2023
a055b18
Merge branch 'main' into obj-inference-warning
T-Gro Jun 13, 2023
ac53fb6
Markups
Smaug123 Jun 14, 2023
7cde8eb
Merge branch 'obj-inference-warning' of github.com:Smaug123/fsharp-1 …
Smaug123 Jun 14, 2023
c1cb455
Merge remote-tracking branch 'origin/main' into obj-inference-warning
Smaug123 Jun 14, 2023
f1404c0
Merge branch 'main' into obj-inference-warning
Smaug123 Jul 4, 2023
8beaa75
Merge branch 'main' into obj-inference-warning
Smaug123 Jul 6, 2023
5e99aae
Merge branch 'obj-inference-warning' of github.com:Smaug123/fsharp-1 …
Smaug123 Jul 6, 2023
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
41 changes: 25 additions & 16 deletions src/Compiler/Checking/TypeRelations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
/// constraint solving and method overload resolution.
module internal FSharp.Compiler.TypeRelations

open FSharp.Compiler.Features
open Internal.Utilities.Collections
open Internal.Utilities.Library
open FSharp.Compiler.DiagnosticsLogger
Expand Down Expand Up @@ -134,50 +135,58 @@ let rec TypeFeasiblySubsumesType ndeep g amap m ty1 canCoerce ty2 =
/// Here x gets a generalized type "list<'T>".
let ChooseTyparSolutionAndRange (g: TcGlobals) amap (tp:Typar) =
let m = tp.Range
let maxTy, m =
let (maxTy, isRefined), m =
let initialTy =
match tp.Kind with
| TyparKind.Type -> g.obj_ty
| TyparKind.Measure -> TType_measure Measure.One
// Loop through the constraints computing the lub
((initialTy, m), tp.Constraints) ||> List.fold (fun (maxTy, _) tpc ->
(((initialTy, false), m), tp.Constraints) ||> List.fold (fun ((maxTy, isRefined), _) tpc ->
let join m x =
if TypeFeasiblySubsumesType 0 g amap m x CanCoerce maxTy then maxTy
elif TypeFeasiblySubsumesType 0 g amap m maxTy CanCoerce x then x
else errorR(Error(FSComp.SR.typrelCannotResolveImplicitGenericInstantiation((DebugPrint.showType x), (DebugPrint.showType maxTy)), m)); maxTy
if TypeFeasiblySubsumesType 0 g amap m x CanCoerce maxTy then maxTy, isRefined
elif TypeFeasiblySubsumesType 0 g amap m maxTy CanCoerce x then x, true
else errorR(Error(FSComp.SR.typrelCannotResolveImplicitGenericInstantiation((DebugPrint.showType x), (DebugPrint.showType maxTy)), m)); maxTy, isRefined
// Don't continue if an error occurred and we set the value eagerly
if tp.IsSolved then maxTy, m else
if tp.IsSolved then (maxTy, isRefined), m else
match tpc with
| TyparConstraint.CoercesTo(x, m) ->
join m x, m
| TyparConstraint.MayResolveMember(_traitInfo, m) ->
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.SimpleChoice(_, m) ->
errorR(Error(FSComp.SR.typrelCannotResolveAmbiguityInPrintf(), m))
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.SupportsNull m ->
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.SupportsComparison m ->
join m g.mk_IComparable_ty, m
| TyparConstraint.SupportsEquality m ->
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.IsEnum(_, m) ->
errorR(Error(FSComp.SR.typrelCannotResolveAmbiguityInEnum(), m))
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.IsDelegate(_, _, m) ->
errorR(Error(FSComp.SR.typrelCannotResolveAmbiguityInDelegate(), m))
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.IsNonNullableStruct m ->
join m g.int_ty, m
| TyparConstraint.IsUnmanaged m ->
errorR(Error(FSComp.SR.typrelCannotResolveAmbiguityInUnmanaged(), m))
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.RequiresDefaultConstructor m ->
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.IsReferenceType m ->
maxTy, m
(maxTy, isRefined), m
| TyparConstraint.DefaultsTo(_priority, _ty, m) ->
maxTy, m)
(maxTy, isRefined), m)

if g.langVersion.SupportsFeature LanguageFeature.DiagnosticForObjInference then
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
match tp.Kind with
| TyparKind.Type ->
if not isRefined then
informationalWarning(Error(FSComp.SR.typrelNeverRefinedAwayFromTop(), m))
| TyparKind.Measure -> ()

maxTy, m

let ChooseTyparSolution g amap tp =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
namespace FSharp.Compiler.ComponentTests.ConstraintSolver

open Xunit
open FSharp.Test.Compiler

module ObjInference =

let message = "A type has been implicitly inferred as 'obj', which may be unintended. Consider adding explicit type annotations. You can disable this warning by using '#nowarn \"3559\"' or '--nowarn:3559'."

let quotableWarningCases =
[
"""System.Object.ReferenceEquals(null, "hello") |> ignore""", 1, 31, 1, 35
"""System.Object.ReferenceEquals("hello", null) |> ignore""", 1, 40, 1, 44
"([] = []) |> ignore", 1, 7, 1, 9
"<@ [] = [] @> |> ignore", 1, 9, 1, 11
"let _ = Unchecked.defaultof<_> in ()", 1, 29, 1, 30
]
|> List.map (fun (str, line1, col1, line2, col2) -> [| box str ; line1 ; col1 ; line2 ; col2 |])

let unquotableWarningCases =
[
"let f() = ([] = [])", 1, 17, 1, 19
"""let f<'b> (x : 'b) : int = failwith ""
let deserialize<'v> (s : string) : 'v = failwith ""
let x = deserialize "" |> f""", 3, 9, 3, 28
"let f = typedefof<_>", 1, 19, 1, 20
"""let f<'b> () : 'b = (let a = failwith "" in unbox a)""", 1, 26, 1, 27
]
|> List.map (fun (str, line1, col1, line2, col2) -> [| box str ; line1 ; col1 ; line2 ; col2 |])

let warningCases =
quotableWarningCases @ unquotableWarningCases

[<Theory>]
[<MemberData(nameof(warningCases))>]
let ``Warning is emitted when type Obj is inferred``(code: string, line1: int, col1: int, line2: int, col2: int) =
FSharp code
|> withErrorRanges
|> withWarnOn 3559
|> withLangVersionPreview
|> typecheck
|> shouldFail
|> withSingleDiagnostic (Information 3559, Line line1, Col col1, Line line2, Col col2, message)

let quotableNoWarningCases =
[
"let a = 5 |> unbox<obj> in let b = a in ()" // explicit obj annotation
"let add x y = x + y in ()" // inferred as int
"let f() = ([] = ([] : obj list)) in ()" // obj is inferred, but is annotated
"let f() = (([] : obj list) = []) in ()" // obj is inferred, but is annotated
"let f () : int = Unchecked.defaultof<_> in ()" // explicitly int
"let f () = Unchecked.defaultof<int> in ()" // explicitly int
]
|> List.map Array.singleton

let unquotableNoWarningCases =
[
"let add x y = x + y" // inferred as int
"let inline add x y = x + y" // inferred with SRTP
"let inline add< ^T when ^T : (static member (+) : ^T * ^T -> ^T)> (x : ^T) (y : ^T) : ^T = x + y" // with SRTP
"let f x = string x" // inferred as generic 'a -> string
"let f() = ([] = ([] : obj list))" // obj is inferred, but is annotated
"let f() = (([] : obj list) = [])" // obj is inferred, but is annotated
"""let x<[<Measure>]'m> : int<'m> = failwith ""
let f () = x = x |> ignore""" // measure is inferred as 1, but that's not covered by this warning
"let f () : int = Unchecked.defaultof<_>" // explicitly int
"let f () = Unchecked.defaultof<int>" // explicitly int
"let f () = Unchecked.defaultof<_>" // generic
]
|> List.map Array.singleton

let noWarningCases = quotableNoWarningCases @ unquotableNoWarningCases

[<Theory>]
[<MemberData(nameof(noWarningCases))>]
let ``Warning does not fire unless required``(code: string) =
FSharp code
|> withWarnOn 3559
|> withLangVersionPreview
|> typecheck
|> shouldSucceed

let nullNoWarningCases =
[
"""System.Object.ReferenceEquals("hello", (null: string))"""
"""System.Object.ReferenceEquals((null: string), "hello")"""
]
|> List.map Array.singleton

[<Theory>]
[<MemberData(nameof(nullNoWarningCases))>]
let ``Don't warn on an explicitly annotated null``(expr: string) =
sprintf "%s |> ignore" expr
|> FSharp
|> withWarnOn 3559
|> withLangVersionPreview
|> typecheck
|> shouldSucceed

[<Theory>]
[<MemberData(nameof(nullNoWarningCases))>]
let ``Don't warn on an explicitly annotated null, inside quotations``(expr: string) =
sprintf "<@ %s @> |> ignore" expr
|> FSharp
|> withWarnOn 3559
|> withLangVersionPreview
|> typecheck
|> shouldSucceed

[<Theory>]
[<MemberData(nameof(quotableWarningCases))>]
let ``Warn also inside quotations of acceptable code``(expr: string, line1: int, col1: int, line2: int, col2: int) =
sprintf "<@ %s @> |> ignore" expr
|> FSharp
|> withWarnOn 3559
|> withLangVersionPreview
|> typecheck
|> shouldFail
|> withSingleDiagnostic (Information 3559, Line line1, Col (col1 + 3), Line line2, Col (col2 + 3), message)

[<Theory>]
[<MemberData(nameof(quotableNoWarningCases))>]
let ``Don't warn inside quotations of acceptable code``(expr: string) =
sprintf "<@ %s @> |> ignore" expr
|> FSharp
|> withWarnOn 3559
|> withLangVersionPreview
|> typecheck
|> shouldSucceed

[<Theory>]
[<MemberData(nameof(warningCases))>]
let ``Warning is off by default``(expr: string, _: int, _: int, _: int, _: int) =
expr
|> FSharp
|> withLangVersionPreview
|> withOptions ["--warnaserror"]
|> typecheck
|> shouldSucceed
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@
<Compile Include="Language\CopyAndUpdateTests.fs" />
<Compile Include="ConstraintSolver\PrimitiveConstraints.fs" />
<Compile Include="ConstraintSolver\MemberConstraints.fs" />
<Compile Include="ConstraintSolver\ObjInference.fs" />
<Compile Include="Interop\DeeplyNestedCSharpClasses.fs" />
<Compile Include="Interop\SimpleInteropTests.fs" />
<Compile Include="Interop\RequiredAndInitOnlyProperties.fs" />
Expand Down
Loading