Skip to content

Commit

Permalink
Turn unused value warnings on by default
Browse files Browse the repository at this point in the history
  • Loading branch information
cartermp committed Nov 26, 2020
1 parent 8f6e2b7 commit 96ec5c5
Show file tree
Hide file tree
Showing 99 changed files with 833 additions and 1,379 deletions.
3 changes: 1 addition & 2 deletions src/fsharp/CompilerDiagnostics.fs
Expand Up @@ -372,8 +372,7 @@ let warningOn err level specificWarnOn =
let n = GetDiagnosticNumber err
List.contains n specificWarnOn ||
// Some specific warnings are never on by default, i.e. unused variable warnings
match n with
| 1182 -> false // chkUnusedValue - off by default
match n with
| 3180 -> false // abImplicitHeapAllocation - off by default
| _ -> level >= GetWarningLevel err

Expand Down
Expand Up @@ -19,7 +19,7 @@ type CreateFSharpManifestResourceName public () =
(linkFileName:string),
(rootNamespace:string), (* may be null *)
(dependentUponFileName:string), (* may be null *)
(binaryStream:System.IO.Stream) (* may be null *)) : string =
(_:System.IO.Stream) (* may be null *)) : string =

// The Visual CSharp and XBuild CSharp toolchains transform resource names like this:
// SubDir\abc.resx --> SubDir.abc.resources
Expand Down
8 changes: 4 additions & 4 deletions src/fsharp/FSharp.Build/FSharpEmbedResourceText.fs
Expand Up @@ -356,7 +356,7 @@ open Printf
printMessage (sprintf "Reading %s" filename)
let lines = File.ReadAllLines(filename)
|> Array.mapi (fun i s -> i,s) // keep line numbers
|> Array.filter (fun (i,s) -> not(s.StartsWith "#")) // filter out comments
|> Array.filter (fun (_,s) -> not(s.StartsWith "#")) // filter out comments

printMessage (sprintf "Parsing %s" filename)
let stringInfos = lines |> Array.map (fun (i,s) -> ParseLine filename i s)
Expand Down Expand Up @@ -391,7 +391,7 @@ open Printf

printMessage (sprintf "Generating resource methods for %s" outFilename)
// gen each resource method
stringInfos |> Seq.iter (fun (lineNum, (optErrNum,ident), str, holes, netFormatString) ->
stringInfos |> Seq.iter (fun (lineNum, (optErrNum,ident), str, holes, _) ->
let formalArgs = new System.Text.StringBuilder()
let actualArgs = new System.Text.StringBuilder()
let mutable firstTime = true
Expand Down Expand Up @@ -428,14 +428,14 @@ open Printf
// gen validation method
fprintfn out " /// Call this method once to validate that all known resources are valid; throws if not"
fprintfn out " static member RunStartupValidation() ="
stringInfos |> Seq.iter (fun (lineNum, (optErrNum,ident), str, holes, netFormatString) ->
stringInfos |> Seq.iter (fun (_, (_,ident), _, _, _) ->
fprintfn out " ignore(GetString(\"%s\"))" ident
)
fprintfn out " ()" // in case there are 0 strings, we need the generated code to parse
// gen to resx
let xd = new System.Xml.XmlDocument()
xd.LoadXml(xmlBoilerPlateString)
stringInfos |> Seq.iter (fun (lineNum, (optErrNum,ident), str, holes, netFormatString) ->
stringInfos |> Seq.iter (fun (_, (_,ident), _, _, netFormatString) ->
let xn = xd.CreateElement("data")
xn.SetAttribute("name",ident) |> ignore
xn.SetAttribute("xml:space","preserve") |> ignore
Expand Down
4 changes: 1 addition & 3 deletions src/fsharp/FSharp.Build/Fsc.fs
Expand Up @@ -63,7 +63,6 @@ type public Fsc () as this =
let mutable tailcalls : bool = true
let mutable targetProfile : string = null
let mutable targetType : string = null
let mutable toolExe : string = "fsc.exe"
let defaultToolPath =
let locationOfThisDll =
try Some(Path.GetDirectoryName(typeof<Fsc>.Assembly.Location))
Expand Down Expand Up @@ -556,7 +555,6 @@ type public Fsc () as this =
match host with
| null -> base.ExecuteTool(pathToTool, responseFileCommands, commandLineCommands)
| _ ->
let sources = sources|>Array.map(fun i->i.ItemSpec)
let invokeCompiler baseCallDelegate =
try
let ret =
Expand All @@ -568,7 +566,7 @@ type public Fsc () as this =
| :? TargetInvocationException as tie when (match tie.InnerException with | :? Microsoft.Build.Exceptions.BuildAbortedException -> true | _ -> false) ->
fsc.Log.LogError(tie.InnerException.Message, [| |])
-1 // ok, this is what happens when VS IDE cancels the build, no need to assert, just log the build-canceled error and return -1 to denote task failed
| e -> reraise()
| _ -> reraise()

let baseCallDelegate = Func<int>(fun () -> fsc.BaseExecuteTool(pathToTool, responseFileCommands, commandLineCommands) )
try
Expand Down
11 changes: 1 addition & 10 deletions src/fsharp/FSharp.Build/Fsi.fs
Expand Up @@ -37,15 +37,12 @@ type public Fsi () as this =
let mutable provideCommandLineArgs = false
let mutable references : ITaskItem[] = [||]
let mutable referencePath : string = null
let mutable resources : ITaskItem[] = [||]
let mutable skipCompilerExecution = false
let mutable sources : ITaskItem[] = [||]
let mutable loadSources : ITaskItem[] = [||]
let mutable useSources : ITaskItem[] = [||]
let mutable tailcalls : bool = true
let mutable targetProfile : string = null
let mutable targetType : string = null
let mutable toolExe : string = "fsi.exe"
let mutable toolPath : string =
let locationOfThisDll =
try Some(Path.GetDirectoryName(typeof<Fsi>.Assembly.Location))
Expand Down Expand Up @@ -85,11 +82,6 @@ type public Fsi () as this =
for item in references do
builder.AppendSwitchIfNotNull("-r:", item.ItemSpec)

let referencePathArray = // create a array of strings
match referencePath with
| null -> null
| _ -> referencePath.Split([|';'; ','|], StringSplitOptions.RemoveEmptyEntries)

// NoWarn
match disabledWarnings with
| null -> ()
Expand Down Expand Up @@ -293,7 +285,6 @@ type public Fsi () as this =
match host with
| null -> base.ExecuteTool(pathToTool, responseFileCommands, commandLineCommands)
| _ ->
let sources = sources|>Array.map(fun i->i.ItemSpec)
let invokeCompiler baseCallDelegate =
try
let ret =
Expand All @@ -305,7 +296,7 @@ type public Fsi () as this =
| :? TargetInvocationException as tie when (match tie.InnerException with | :? Microsoft.Build.Exceptions.BuildAbortedException -> true | _ -> false) ->
fsi.Log.LogError(tie.InnerException.Message, [| |])
-1 // ok, this is what happens when VS IDE cancels the build, no need to assert, just log the build-canceled error and return -1 to denote task failed
| e -> reraise()
| _ -> reraise()

let baseCallDelegate = Func<int>(fun () -> fsi.BaseExecuteTool(pathToTool, responseFileCommands, commandLineCommands) )
try
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/FSharp.Build/MapSourceRoots.fs
Expand Up @@ -122,7 +122,7 @@ type MapSourceRoots () =

for root in mappedSourceRoots do
match root.GetMetadata SourceControl with
| HasValue v when isSourceControlled -> mapNestedRootIfEmpty root
| HasValue _ when isSourceControlled -> mapNestedRootIfEmpty root
| NullOrEmpty when not isSourceControlled -> mapNestedRootIfEmpty root
| _ -> ()

Expand Down
Expand Up @@ -9,7 +9,7 @@
<AssemblyName>FSharp.Compiler.Interactive.Settings</AssemblyName>
<NoWarn>$(NoWarn);45;55;62;75;1182;1204</NoWarn>
<AllowCrossTargeting>true</AllowCrossTargeting>
<OtherFlags>$(OtherFlags) --maxerrors:20 --extraoptimizationloops:1</OtherFlags>
<OtherFlags>$(OtherFlags) --maxerrors:20 --extraoptimizationloops:1 --warnaserror-:1182</OtherFlags>
<NGenBinary>true</NGenBinary>
</PropertyGroup>

Expand Down
Expand Up @@ -41,7 +41,7 @@ type internal FSharpInteractiveServer() =
LifetimeServices.RenewOnCallTime <- TimeSpan(7,0,0,0);
LifetimeServices.SponsorshipTimeout <- TimeSpan(7,0,0,0);
ChannelServices.RegisterChannel(chan,false);
let objRef = RemotingServices.Marshal(server,"FSIServer")
RemotingServices.Marshal(server,"FSIServer") |> ignore
()

static member StartClient(channelName) =
Expand Down
10 changes: 5 additions & 5 deletions tests/FSharp.Build.UnitTests/MapSourceRootsTests.fs
Expand Up @@ -14,7 +14,7 @@ type MockEngine() =
member val Messages = ResizeArray() with get

interface IBuildEngine with
member this.BuildProjectFile(projectFileName: string, targetNames: string [], globalProperties: System.Collections.IDictionary, targetOutputs: System.Collections.IDictionary): bool =
member this.BuildProjectFile(_: string, _: string [], _: System.Collections.IDictionary, _: System.Collections.IDictionary): bool =
failwith "Not Implemented"
member this.ColumnNumberOfTaskNode: int = 0
member this.ContinueOnError: bool = true
Expand Down Expand Up @@ -141,7 +141,7 @@ type MapSourceRootsTests() =
Assert.IsTrue(errorMessages |> Seq.exists (fun err -> err.EndsWith expectedErrorPath),
sprintf "expected an error to end with '%s', none did.\nMessages were:\n%A" expectedErrorPath errorMessages)
)
| Some mappings ->
| Some _ ->
Assert.Fail("Expected to fail on the inputs")

[<Test>]
Expand Down Expand Up @@ -219,7 +219,7 @@ type MapSourceRootsTests() =
|> Seq.iter (fun expectedErrorPath ->
Assert.IsTrue(errorMessages |> Seq.exists (fun err -> err.EndsWith expectedErrorPath), sprintf "expected an error to end with '%s', none did.\nMessages were:\n%A" expectedErrorPath errorMessages)
)
| Some mappings ->
| Some _ ->
Assert.Fail("Expected to fail on the inputs")

[<TestCase(true)>]
Expand Down Expand Up @@ -348,7 +348,7 @@ type MapSourceRootsTests() =
|> Seq.iter (fun expectedErrorPath ->
Assert.IsTrue(errorMessages |> Seq.exists (fun err -> err.EndsWith expectedErrorPath), sprintf "expected an error to end with '%s', none did.\nMessages were:\n%A" expectedErrorPath errorMessages)
)
| Some mappings ->
| Some _ ->
Assert.Fail("Expected to fail on the inputs")

[<Test>]
Expand Down Expand Up @@ -379,7 +379,7 @@ type MapSourceRootsTests() =
|> Seq.iter (fun expectedErrorPath ->
Assert.IsTrue(errorMessages |> Seq.exists (fun err -> err.EndsWith expectedErrorPath), sprintf "expected an error to end with '%s', none did.\nMessages were:\n%A" expectedErrorPath errorMessages)
)
| Some mappings ->
| Some _ ->
Assert.Fail("Expected to fail on the inputs")

[<TestCase(true)>]
Expand Down
Expand Up @@ -82,8 +82,9 @@ let changeProperty() =
"""
|> typecheck
|> shouldFail
|> withSingleDiagnostic (Warning 20, Line 9, Col 5, Line 9, Col 23,
"The result of this equality expression has type 'bool' and is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'.")
|> withDiagnostics [
(Warning 20, Line 9, Col 5, Line 9, Col 23, "The result of this equality expression has type 'bool' and is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'.")
(Warning 1182, Line 2, Col 14, Line 2, Col 23, "The value 'property1' is unused")]

[<Fact>]
let ``Warn If Implicitly Discarded``() =
Expand Down
Expand Up @@ -18,6 +18,10 @@
<Content Include="resources\**" CopyToOutputDirectory="Never" CopyToPublishDirectory="Never" />
</ItemGroup>

<ItemGroup>
<EmbeddedResource Remove="NewFolder\**" />
</ItemGroup>

<ItemGroup>
<Compile Include="EmittedIL\Operators.fs" />
<Compile Include="EmittedIL\Literals.fs" />
Expand Down Expand Up @@ -57,8 +61,4 @@
<ProjectReference Include="$(FSharpSourcesRoot)\fsharp\FSharp.Core\FSharp.Core.fsproj" />
<ProjectReference Include="$(FSharpTestsRoot)\FSharp.Test.Utilities\FSharp.Test.Utilities.fsproj" />
</ItemGroup>

<ItemGroup>
<Folder Include="NewFolder\" />
</ItemGroup>
</Project>
Expand Up @@ -34,11 +34,6 @@ type DependencyManagerInteractiveTests() =
| Ok(value) -> value
| Error ex -> raise ex

let getErrors ((_value: Result<FsiValue option, exn>), (errors: FSharpErrorInfo[])) =
errors

let ignoreValue = getValue >> ignore

[<Fact>]
member __.``SmokeTest - #r nuget``() =
let text = """
Expand All @@ -56,7 +51,7 @@ type DependencyManagerInteractiveTests() =
#r @"nuget:System.Collections.Immutable.DoesNotExist, version=1.5.0"
0"""
use script = new scriptHost()
let opt, errors = script.Eval(text)
let _, errors = script.Eval(text)
Assert.Equal(errors.Length, 1)

(*
Expand Down Expand Up @@ -151,8 +146,6 @@ type DependencyManagerInteractiveTests() =

[<Fact>]
member __.``Multiple Instances of DependencyProvider should be isolated``() =

let assemblyProbingPaths () = Seq.empty<string>
let nativeProbingRoots () = Seq.empty<string>

use dp1 = new DependencyProvider(NativeResolutionProbe(nativeProbingRoots))
Expand Down Expand Up @@ -277,7 +270,6 @@ TorchSharp.Tensor.LongTensor.From([| 0L .. 100L |]).Device
| ErrorReportType.Warning -> printfn "PackageManagementWarning %d : %s" code message
ResolvingErrorReport (report)

let mutable resolverPackageRoots = Seq.empty<string>
let mutable resolverPackageRoots = Seq.empty<string>
let mutable resolverReferences = Seq.empty<string>

Expand Down Expand Up @@ -374,11 +366,9 @@ printfn ""%A"" result
ResolvingErrorReport (report)

let mutable resolverPackageRoots = Seq.empty<string>
let mutable resolverPackageRoots = Seq.empty<string>

let mutable resolverReferences = Seq.empty<string>
let nativeProbingRoots () = resolverPackageRoots
let assemblyPaths () = resolverReferences

// Restore packages, Get Reference dll paths and package roots
let result =
Expand Down Expand Up @@ -513,10 +503,8 @@ x |> Seq.iter(fun r ->
ResolvingErrorReport (report)

let mutable resolverPackageRoots = Seq.empty<string>
let mutable resolverReferences = Seq.empty<string>

let nativeProbingRoots () = resolverPackageRoots
let assemblyProbingPaths () = resolverReferences

// Restore packages, Get Reference dll paths and package roots
let result =
Expand All @@ -539,10 +527,8 @@ x |> Seq.iter(fun r ->
ResolvingErrorReport (report)

let mutable resolverPackageRoots = Seq.empty<string>
let mutable resolverReferences = Seq.empty<string>

let nativeProbingRoots () = resolverPackageRoots
let assemblyProbingPaths () = resolverReferences

// Restore packages, Get Reference dll paths and package roots
let result =
Expand Down Expand Up @@ -645,7 +631,7 @@ x |> Seq.iter(fun r ->

// Set up AssemblyResolver to resolve dll's
do
use dp = new AssemblyResolveHandler(AssemblyResolutionProbe(assemblyProbingPaths))
use __ = new AssemblyResolveHandler(AssemblyResolutionProbe(assemblyProbingPaths))

// Invoking a non-existent assembly causes a probe. which should invoke the call back
try Assembly.Load("NoneSuchAssembly") |> ignore with _ -> ()
Expand Down Expand Up @@ -741,16 +727,15 @@ x |> Seq.iter(fun r ->
lines.Add(line)
match expected |> Array.tryFind(compareLine) with
| None -> ()
| Some t ->
| Some _ ->
found <- found + 1
if found = expected.Length then sawExpectedOutput.Set() |> ignore

let text = "#help"
use output = new RedirectConsoleOutput()
use script = new FSharpScript(quiet = false, langVersion = LangVersion.V47)
let mutable found = 0
output.OutputProduced.Add (fun line -> verifyOutput line)
let opt = script.Eval(text) |> getValue
let _ = script.Eval(text) |> getValue
Assert.True(sawExpectedOutput.WaitOne(TimeSpan.FromSeconds(5.0)), sprintf "Expected to see error sentinel value written\nexpected:%A\nactual:%A" expected lines)


Expand Down Expand Up @@ -790,14 +775,13 @@ x |> Seq.iter(fun r ->
lines.Add(line)
match expected |> Array.tryFind(compareLine) with
| None -> ()
| Some t ->
| Some _ ->
found <- found + 1
if found = expected.Length then sawExpectedOutput.Set() |> ignore

let text = "#help"
use output = new RedirectConsoleOutput()
use script = new FSharpScript(quiet = false, langVersion = LangVersion.Preview)
let mutable found = 0
output.OutputProduced.Add (fun line -> verifyOutput line)
let opt = script.Eval(text) |> getValue
let _ = script.Eval(text) |> getValue
Assert.True(sawExpectedOutput.WaitOne(TimeSpan.FromSeconds(5.0)), sprintf "Expected to see error sentinel value written\nexpected:%A\nactual:%A" expected lines)

0 comments on commit 96ec5c5

Please sign in to comment.