Skip to content

Commit a4ec896

Browse files
authored
Warn if unused top-level function is not included is signature file. (#14567)
* Warn is unused toplevel function is not included is signature file. * Add HasSignature to Val and Symbol.fs * Verify the parent of the value has a signature file. * Scope warning to let bindings only. * Apply feedback from code review. * Surface Area Release mode * Add test for type extension.
1 parent 47a4241 commit a4ec896

File tree

9 files changed

+149
-6
lines changed

9 files changed

+149
-6
lines changed

src/Compiler/Checking/PostInferenceChecks.fs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,12 +302,24 @@ let BindVal cenv env (v: Val) =
302302
//printfn "binding %s..." v.DisplayName
303303
let alreadyDone = cenv.boundVals.ContainsKey v.Stamp
304304
cenv.boundVals[v.Stamp] <- 1
305+
306+
let topLevelBindingHiddenBySignatureFile () =
307+
let parentHasSignatureFile () =
308+
match v.TryDeclaringEntity with
309+
| ParentNone -> false
310+
| Parent p ->
311+
match p.TryDeref with
312+
| ValueNone -> false
313+
| ValueSome e -> e.HasSignatureFile
314+
315+
v.IsModuleBinding && not v.HasSignatureFile && parentHasSignatureFile ()
316+
305317
if not env.external &&
306318
not alreadyDone &&
307319
cenv.reportErrors &&
308320
not v.HasBeenReferenced &&
309-
not v.IsCompiledAsTopLevel &&
310-
not (v.DisplayName.StartsWithOrdinal("_")) &&
321+
(not v.IsCompiledAsTopLevel || topLevelBindingHiddenBySignatureFile ()) &&
322+
not (v.DisplayName.StartsWithOrdinal("_")) &&
311323
not v.IsCompilerGenerated then
312324

313325
if v.IsCtorThisVal then

src/Compiler/Service/FSharpCheckerResults.fs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,7 @@ type FSharpSymbolUse(denv: DisplayEnv, symbol: FSharpSymbol, inst: TyparInstanti
270270

271271
let fileHasSignatureFile = fileSignatureLocation <> fileDeclarationLocation
272272

273-
let symbolIsNotInSignatureFile = m.SignatureLocation = Some m.DeclarationLocation
274-
275-
fileHasSignatureFile && symbolIsNotInSignatureFile
273+
fileHasSignatureFile && not m.HasSignatureFile
276274
|| not m.IsModuleValueOrMember
277275
|| m.Accessibility.IsPrivate
278276
| :? FSharpEntity as m -> m.Accessibility.IsPrivate

src/Compiler/Symbols/Symbols.fs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1826,6 +1826,14 @@ type FSharpMemberOrFunctionOrValue(cenv, d:FSharpMemberOrValData, item) =
18261826
| P _ -> true
18271827
| _ -> false
18281828

1829+
member x.HasSignatureFile =
1830+
match fsharpInfo() with
1831+
| None -> false
1832+
| Some vref ->
1833+
match vref.TryDeref with
1834+
| ValueNone -> false
1835+
| ValueSome v -> v.HasSignatureFile
1836+
18291837
member _.IsEvent =
18301838
match d with
18311839
| E _ -> true

src/Compiler/Symbols/Symbols.fsi

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,9 @@ type FSharpMemberOrFunctionOrValue =
816816
/// Indicates if this is a method member
817817
member IsMethod: bool
818818

819+
/// Indicates if the value has a signature file counterpart
820+
member HasSignatureFile: bool
821+
819822
/// Indicates if this is a property and there exists an associated getter method
820823
member HasGetterMethod: bool
821824

src/Compiler/TypedTree/TypedTree.fs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,6 +1276,8 @@ type Entity =
12761276

12771277
/// Indicates if we have pre-determined that a type definition has a self-referential constructor using 'as x'
12781278
member x.HasSelfReferentialConstructor = x.entity_flags.HasSelfReferentialConstructor
1279+
1280+
member x.HasSignatureFile = x.SigRange <> x.DefinitionRange
12791281

12801282
/// Set the custom attributes on an F# type definition.
12811283
member x.SetAttribs attribs = x.entity_attribs <- attribs
@@ -2803,6 +2805,9 @@ type Val =
28032805
// Indicates if this value was declared to be a type function, e.g. "let f<'a> = typeof<'a>"
28042806
member x.IsTypeFunction = x.val_flags.IsTypeFunction
28052807

2808+
member x.HasSignatureFile =
2809+
x.SigRange <> x.DefinitionRange
2810+
28062811
/// Get the inline declaration on the value
28072812
member x.InlineInfo = x.val_flags.InlineInfo
28082813

src/Compiler/TypedTree/TypedTree.fsi

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,9 @@ type Entity =
571571
/// Indicates if we have pre-determined that a type definition has a self-referential constructor using 'as x'
572572
member HasSelfReferentialConstructor: bool
573573

574+
/// Indicates if the value has a signature file counterpart
575+
member HasSignatureFile: bool
576+
574577
/// Get the Abstract IL scope, nesting type metadata for this
575578
/// type definition, assuming it is backed by Abstract IL metadata.
576579
member ILTyconInfo: TILObjectReprData
@@ -2085,6 +2088,9 @@ type Val =
20852088

20862089
member IsTypeFunction: bool
20872090

2091+
/// Indicates if the value has a signature file counterpart
2092+
member HasSignatureFile: bool
2093+
20882094
/// The value of a value or member marked with [<LiteralAttribute>]
20892095
member LiteralValue: Const option
20902096

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,111 @@ module warnon =
3434
|> withDiagnosticMessageMatches "The value 'n' is unused$"
3535
|> ignore
3636

37+
[<Fact>]
38+
let ``warnon unused public function hidden by signature file`` () =
39+
let signatureFile: SourceCodeFileKind =
40+
SourceCodeFileKind.Create(
41+
"Library.fsi",
42+
"""
43+
module Foo
44+
45+
val add: a:int -> b:int -> int
46+
""" )
47+
48+
let implementationFile =
49+
SourceCodeFileKind.Create(
50+
"Library.fs",
51+
"""
52+
module Foo
53+
54+
let add a b = a + b
55+
let subtract a b = a - b
56+
""" )
57+
58+
fsFromString signatureFile
59+
|> FS
60+
|> withAdditionalSourceFile implementationFile
61+
|> withOptions ["--warnon:FS1182"]
62+
|> asLibrary
63+
|> compile
64+
|> withWarningCode 1182
65+
|> withDiagnosticMessageMatches "The value 'subtract' is unused$"
66+
|> ignore
67+
68+
[<Fact>]
69+
let ``Don't warnon unused public function`` () =
70+
let implementationFile =
71+
SourceCodeFileKind.Create(
72+
"Library.fs",
73+
"""
74+
module Foo
75+
76+
let add a b = a + b
77+
let subtract a b = a - b
78+
""" )
79+
80+
fsFromString implementationFile
81+
|> FS
82+
|> withOptions ["--warnon:FS1182"]
83+
|> asLibrary
84+
|> compile
85+
|> withDiagnostics []
86+
|> ignore
87+
88+
[<Fact>]
89+
let ``Don't warnon unused public function hidden by signature file that starts with an underscore`` () =
90+
let signatureFile: SourceCodeFileKind =
91+
SourceCodeFileKind.Create(
92+
"Library.fsi",
93+
"""
94+
module Foo
95+
96+
val add: a:int -> b:int -> int
97+
""" )
98+
99+
let implementationFile =
100+
SourceCodeFileKind.Create(
101+
"Library.fs",
102+
"""
103+
module Foo
104+
105+
let add a b = a + b
106+
let _subtract a b = a - b
107+
""" )
108+
109+
fsFromString signatureFile
110+
|> FS
111+
|> withAdditionalSourceFile implementationFile
112+
|> withOptions ["--warnon:FS1182"]
113+
|> asLibrary
114+
|> compile
115+
|> withDiagnostics []
116+
|> ignore
117+
118+
[<Fact>]
119+
let ``Type extensions are not included in this warnon check`` () =
120+
let signatureFile: SourceCodeFileKind =
121+
SourceCodeFileKind.Create(
122+
"Library.fsi",
123+
"""
124+
module Foo
125+
""" )
126+
127+
let implementationFile =
128+
SourceCodeFileKind.Create(
129+
"Library.fs",
130+
"""
131+
module Foo
132+
133+
type System.Int32 with
134+
member x.Bar () = x + 1
135+
""" )
136+
137+
fsFromString signatureFile
138+
|> FS
139+
|> withAdditionalSourceFile implementationFile
140+
|> withOptions ["--warnon:FS1182"]
141+
|> asLibrary
142+
|> compile
143+
|> shouldSucceed
144+
|> ignore

tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2012,7 +2012,6 @@ FSharp.Compiler.CodeAnalysis.FSharpCheckProjectResults: FSharp.Compiler.Symbols.
20122012
FSharp.Compiler.CodeAnalysis.FSharpCheckProjectResults: System.String ToString()
20132013
FSharp.Compiler.CodeAnalysis.FSharpCheckProjectResults: System.String[] DependencyFiles
20142014
FSharp.Compiler.CodeAnalysis.FSharpCheckProjectResults: System.String[] get_DependencyFiles()
2015-
FSharp.Compiler.CodeAnalysis.FSharpChecker
20162015
FSharp.Compiler.CodeAnalysis.FSharpChecker: FSharp.Compiler.CodeAnalysis.FSharpChecker Create(Microsoft.FSharp.Core.FSharpOption`1[System.Int32], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean], Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.CodeAnalysis.LegacyReferenceResolver], Microsoft.FSharp.Core.FSharpOption`1[Microsoft.FSharp.Core.FSharpFunc`2[System.Tuple`2[System.String,System.DateTime],Microsoft.FSharp.Core.FSharpOption`1[System.Tuple`3[System.Object,System.IntPtr,System.Int32]]]], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean], Microsoft.FSharp.Core.FSharpOption`1[System.Boolean])
20172016
FSharp.Compiler.CodeAnalysis.FSharpChecker: FSharp.Compiler.CodeAnalysis.FSharpChecker Instance
20182017
FSharp.Compiler.CodeAnalysis.FSharpChecker: FSharp.Compiler.CodeAnalysis.FSharpChecker get_Instance()
@@ -4782,6 +4781,7 @@ FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean Equals(System.Obj
47824781
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean EventIsStandard
47834782
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean HasGetterMethod
47844783
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean HasSetterMethod
4784+
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean HasSignatureFile
47854785
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean IsActivePattern
47864786
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean IsBaseValue
47874787
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean IsCompilerGenerated
@@ -4814,6 +4814,7 @@ FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean IsValue
48144814
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_EventIsStandard()
48154815
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_HasGetterMethod()
48164816
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_HasSetterMethod()
4817+
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_HasSignatureFile()
48174818
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_IsActivePattern()
48184819
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_IsBaseValue()
48194820
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_IsCompilerGenerated()

tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4781,6 +4781,7 @@ FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean Equals(System.Obj
47814781
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean EventIsStandard
47824782
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean HasGetterMethod
47834783
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean HasSetterMethod
4784+
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean HasSignatureFile
47844785
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean IsActivePattern
47854786
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean IsBaseValue
47864787
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean IsCompilerGenerated
@@ -4813,6 +4814,7 @@ FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean IsValue
48134814
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_EventIsStandard()
48144815
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_HasGetterMethod()
48154816
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_HasSetterMethod()
4817+
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_HasSignatureFile()
48164818
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_IsActivePattern()
48174819
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_IsBaseValue()
48184820
FSharp.Compiler.Symbols.FSharpMemberOrFunctionOrValue: Boolean get_IsCompilerGenerated()

0 commit comments

Comments
 (0)