Required 'new' keyword marked as redundant [new in 0.12.3]

[SteveGilham]

Description

Statements of the form use name = new IDisposable() now get marked with redundant new lint messages, as do some let name = new IDisposable() where the disposal happens further up the calling stack; this happens after the 0.12.2 to 0.12.3 upgrade

Repro steps

Code like this now gets flagged

    try
      Output.Error <- CommandLine.WriteErr
      Output.Info <- CommandLine.WriteOut
      use stdout = new StringWriter()
      use stderr = new StringWriter()
      Console.SetOut stdout
      Console.SetError stderr

as does a module-level definition

  let latch = new Threading.ManualResetEvent false

In each case, removing the new provokes "Error FS0760 It is recommended that objects supporting the IDisposable interface are created using the syntax 'new Type(args)', rather than 'Type(args)' or 'Type' as a function value representing the constructor, to indicate that resources may be owned by the generated value" in Visual Studio

Expected behavior

At 0.12.2, no lint messages were raised for these statements, nor should there be.

Actual behavior

At 0.12.3, "redundant new" lint messages were omitted

Known workarounds

Disabling the rule with "redundantNewKeyword": { "enabled": false },

Related information

• Operating system : Windows 10 Home
• Branch : NuGet 0.12.3 package
• .NET Runtime, CoreCLR or Mono Version : .net SDK 2.1.701

[jgardella]

Hi @SteveGilham, thanks for the report.

I'm not able to reproduce this using the examples you posted. Could you provide a link to the full code for these examples if possible? This is one of the few rules that depends on type checker information (to determine if a type implements IDisposable). My suspicion is that some other code in the file is causing the typechecker to fail.

[SteveGilham]

Sure thing

https://github.com/SteveGilham/altcover/blob/7db21cf94d9d7cd20e65295da72d1015505c2686/XTests/XTests.fs#L438-L439 https://github.com/SteveGilham/altcover/blob/7db21cf94d9d7cd20e65295da72d1015505c2686/XTests/XTests.fs#L625-L626

and

https://github.com/SteveGilham/altcover/blob/7db21cf94d9d7cd20e65295da72d1015505c2686/AltCover.Visualizer/Visualizer.fs#L876

Driving the linter directly from Fake as at https://github.com/SteveGilham/altcover/blob/7db21cf94d9d7cd20e65295da72d1015505c2686/Build/targets.fsx#L291-L323

[jgardella]

@SteveGilham, I still haven't been able to reproduce this. I copied your entire XTests.fs file contents into a test running just the RedundantNewKeyword rule, and it still didn't produce any errors. If you could help me to create a minimal reproduction of this bug that would be much appreciated!

[SteveGilham]

Here's one of the problem cases, that of the module level

  let latch = new Threading.ManualResetEvent false

cut right down to be getting on with

altcover.zip

If the Visualizer.fs source file alone doesn't do it, then

dotnet restore .\dotnet-fake.fsproj
dotnet fake run .\Build\Build.fsx

repeatably results in

Starting target 'Lint': Lint
Info: "FL0014: Usage of `new` keyword here is redundant."
 Range: C:\Users\steve\Documents\GitHub\altcover\AltCover.Visualizer\Visualizer.fs (867,14--867,50) IsSynthetic=false    Fix: Some
  { FromText = "new Threading.ManualResetEvent false"
    FromRange =
               C:\Users\steve\Documents\GitHub\altcover\AltCover.Visualizer\Visualizer.fs (867,14--867,50) IsSynthetic=false
    ToText = "Threading.ManualResetEvent false" }
====
System.InvalidOperationException: Lint issues were found
   at [email protected](Boolean issuesFound) in C:\Users\steve\Documents\GitHub\altcover\Build\targets.fsx:line 29
   at Targets.f@23(TargetParameter _arg2) in C:\Users\steve\Documents\GitHub\altcover\Build\targets.fsx:line 40

[SteveGilham]

And here's the other example, cut right down. altcover.zip Same

dotnet restore .\dotnet-fake.fsproj
dotnet fake run .\Build\Build.fsx

if needed, and produces

Info: "FL0014: Usage of `new` keyword here is redundant."
 Range: C:\Users\steve\Documents\GitHub\altcover\XTests\XTests.fs (25,19--25,37) IsSynthetic=false
 Fix: Some
  { FromText = "new StringWriter()"
    FromRange =
               C:\Users\steve\Documents\GitHub\altcover\XTests\XTests.fs (25,19--25,37) IsSynthetic=false
    ToText = "StringWriter()" }
====
Info: "FL0014: Usage of `new` keyword here is redundant."
 Range: C:\Users\steve\Documents\GitHub\altcover\XTests\XTests.fs (24,19--24,37) IsSynthetic=false
 Fix: Some
  { FromText = "new StringWriter()"
    FromRange =
               C:\Users\steve\Documents\GitHub\altcover\XTests\XTests.fs (24,19--24,37) IsSynthetic=false
    ToText = "StringWriter()" }
====
System.InvalidOperationException: Lint issues were found

[milbrandt]

I have the issue, that the message is no longer found when I update from FSharp.Core 4.6.2 to 4.7.0. Code example is unit testing analyzing this file. Updating to FSharpLint.Core 0.12.3 didn't Change behaviour.

@SteveGilham Which release of FSharp are you using?

[SteveGilham]

I'm driving the linting with FAKE 5.16.0, which takes FSharp 4.7 as a dependency. The case of

  let latch = new Threading.ManualResetEvent false

is in a project that also takes FSharp 4.7 as a dependency.

[milbrandt]

A reduced example of my no longer reported 'new' when updating FSharp.Core from 4.6.2 to 4.7.0 I've provided at https://github.com/milbrandt/RedundantNewFSharp470

The update of FsharpLint to latest 0.13.1 didn't help (but required some work with the breaking changes in 0.12.7 and 0.13.0)