Skip to content
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@
<Compile Include="src\QuickFixes\AddExtensionAttributeFix.fs" />
<Compile Include="src\QuickFixes\RemoveRedundantParenExprFix.fs" />
<Compile Include="src\QuickFixes\NamespaceToModuleFix.fs" />
<Compile Include="src\QuickFixes\IgnoreUnusedBindingExpressionFix.fs" />
<Compile Include="src\PostfixTemplates\PostfixTemplates.fs" />
<Compile Include="src\PostfixTemplates\NotTemplate.fs" />
<Compile Include="src\PostfixTemplates\LetPostfixTemplate.fs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
<QuickFix>AddUnderscorePrefixFix</QuickFix>
<QuickFix>RemoveUnusedLocalBindingFix</QuickFix>
<QuickFix>RemoveUnusedNamedAsPatFix</QuickFix>
<QuickFix>IgnoreUnusedBindingExpressionFix</QuickFix>
</Warning>

<Warning staticGroup="FSharpErrors" name="UnitTypeExpected">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
namespace JetBrains.ReSharper.Plugins.FSharp.Psi.Features.Daemon.QuickFixes

open JetBrains.ReSharper.Plugins.FSharp.Psi.Features.Daemon.Highlightings
open JetBrains.ReSharper.Plugins.FSharp.Psi.Impl
open JetBrains.ReSharper.Plugins.FSharp.Psi.Impl.Tree
open JetBrains.ReSharper.Plugins.FSharp.Psi.Tree
open JetBrains.ReSharper.Plugins.FSharp.Psi
open JetBrains.ReSharper.Plugins.FSharp.Psi.Features.Util
open JetBrains.ReSharper.Psi
open JetBrains.ReSharper.Psi.ExtensionsAPI.Tree
open JetBrains.ReSharper.Resources.Shell

type IgnoreUnusedBindingExpressionFix(warning: UnusedValueWarning) =
inherit FSharpQuickFixBase()

let pat = warning.Pat.IgnoreParentParens()
let binding = BindingNavigator.GetByHeadPattern(pat)
let letOrUseExpr = LetOrUseExprNavigator.GetByBinding(binding)

let ignoreInnermostExpression (expr: IFSharpExpression) =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to move it to something like FSharpExpressionUtil.

let rec getInnermostExpression (expr: IFSharpExpression) =
match expr with
| :? ISequentialExpr as seqExpr -> getInnermostExpression (seqExpr.Expressions.Last())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExpressionsEnumerable is a bit preferable since it doesn't store the whole collection.

| :? ILetOrUseExpr as letOrUseExpr -> getInnermostExpression letOrUseExpr.InExpression
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InExpression can be null.

| :? IParenExpr as parenExpr -> getInnermostExpression parenExpr.InnerExpression
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above.

| _ -> expr

let exprToIgnore = getInnermostExpression expr
let ignoredExpr = exprToIgnore.CreateElementFactory().CreateIgnoreApp(exprToIgnore, false)
let replaced = ModificationUtil.ReplaceChild(exprToIgnore, ignoredExpr)
addParensIfNeeded replaced.LeftArgument |> ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be easy to unify it with AddIgnoreFix?


override x.Text = "Inline and ignore expression"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just Ignore expression?


override x.IsAvailable _ =
isValid pat && isValid letOrUseExpr && letOrUseExpr.Bindings.Count = 1 && isValid binding.Expression
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it'll work on function bindings (and it shouldn't):

do
  let f{caret} _ = ()
  ()

&& not (binding.Expression :? IDoExpr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move && to the previous line.


override x.ExecutePsiTransaction _ =
use writeLock = WriteLockCookie.Create(pat.IsPhysical())
use formatter = FSharpRegistryUtil.AllowFormatterCookie.Create()

if not (binding.Expression.Type().IsVoid()) then
ignoreInnermostExpression binding.Expression

let inExpr = letOrUseExpr.InExpression
let newLine = NewLine(letOrUseExpr.GetLineEnding())

let bindingExpr = ModificationUtil.ReplaceChild(letOrUseExpr, binding.Expression)
addNodesAfter bindingExpr [
newLine
newLine
inExpr
] |> ignore
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,22 @@ private void Aligning()

private void Formatting()
{
Describe<FormattingRule>()
.Name("LineBreakBeforeIgnorePipe")
.Group(LineBreaksRuleGroup)
.Where(
Parent()
.HasType(ElementType.BINARY_APP_EXPR)
.Satisfies((node, context) => ((IBinaryAppExpr) node).RightArgument.GetText() == "ignore"),
Left()
.HasRole(BinaryAppExpr.LEFT_ARG_EXPR)
.Satisfies((node, context) => node.ContainsLineBreak(context.CodeFormatter))
.In(ElementType.MATCH_EXPR, ElementType.IF_THEN_ELSE_EXPR, ElementType.TRY_WITH_EXPR,
ElementType.TRY_FINALLY_EXPR, ElementType.LAMBDA_EXPR, ElementType.ASSERT_EXPR,
ElementType.LAZY_EXPR, ElementType.MATCH_LAMBDA_EXPR))
.Return(IntervalFormatType.NewLine)
.Build();

Describe<FormattingRule>()
.Group(SpaceRuleGroup)
.Name("SpaceAfterImplicitConstructorDecl")
Expand Down Expand Up @@ -285,7 +301,8 @@ private void DescribeSimpleAlignmentRule((string name, CompositeNodeType nodeTyp
}

private static bool IndentElseExpr(ITreeNode elseExpr, CodeFormattingContext context) =>
elseExpr.GetPreviousMeaningfulSibling().IsFirstOnLine(context.CodeFormatter) && !(elseExpr is IElifExpr);
(elseExpr.GetPreviousMeaningfulSibling().IsFirstOnLine(context.CodeFormatter)
|| !AreAligned(elseExpr, elseExpr.Parent?.FirstChild, context.CodeFormatter)) && !(elseExpr is IElifExpr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move || to the previous line.


private static bool AreAligned(ITreeNode first, ITreeNode second, IWhitespaceChecker whitespaceChecker) =>
first.CalcLineIndent(whitespaceChecker) == second.CalcLineIndent(whitespaceChecker);
Expand Down
9 changes: 9 additions & 0 deletions ReSharper.FSharp/src/FSharp.Psi/src/Impl/Tree/AssertExpr.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using JetBrains.ReSharper.Psi;

namespace JetBrains.ReSharper.Plugins.FSharp.Psi.Impl.Tree
{
internal partial class AssertExpr
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please inherit from UnitExpressionBase instead.

{
public override IType Type() => GetPsiModule().GetPredefinedType().Void;
}
}
3 changes: 3 additions & 0 deletions ReSharper.FSharp/src/FSharp.Psi/src/Impl/Tree/LetOrUseExpr.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using JetBrains.Diagnostics;
using JetBrains.ReSharper.Plugins.FSharp.Psi.Parsing;
using JetBrains.ReSharper.Psi;

namespace JetBrains.ReSharper.Plugins.FSharp.Psi.Impl.Tree
{
Expand All @@ -18,5 +19,7 @@ public void SetIsRecursive(bool value)

LetOrUseToken.NotNull().AddTokenAfter(FSharpTokenType.REC);
}

public override IType Type() => InExpression.Type();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using JetBrains.ReSharper.Psi;

namespace JetBrains.ReSharper.Plugins.FSharp.Psi.Impl.Tree
{
internal partial class TryFinallyExpr
{
public override IType Type() => GetPsiModule().GetPredefinedType().Void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let a: int =
    try 1
    finally ()

}
}
9 changes: 9 additions & 0 deletions ReSharper.FSharp/src/FSharp.Psi/src/Impl/Tree/TryWithExpr.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using JetBrains.ReSharper.Psi;

namespace JetBrains.ReSharper.Plugins.FSharp.Psi.Impl.Tree
{
internal partial class TryWithExpr
{
public override IType Type() => GetPsiModule().GetPredefinedType().Void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let a: int =
    try 1
    with _ -> 1

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Replace with '_'
--Replace unused values with '_' in solution
Rename to '_foo'
Remove unused value
Inline and ignore expression
1: The value 'foo bar' is unused
QUICKFIXES:
Replace with '_'
Expand All @@ -23,3 +24,4 @@ Replace with '_'
--Replace unused values with '_' in project
--Replace unused values with '_' in solution
Remove unused value
Inline and ignore expression
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Module

let a =
Copy link
Member

@auduchinok auduchinok Aug 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could save up to a half of the dump by using do statement instead of the top-level binding.

module Module

do
  let a =
    1
    1

  ()

A bonus point would be replacing the top-level module with a anon module, but it'd require more changes in test project options, so I'd rather do it separately.

let b{caret} =
1
1

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module Module

let a =
1{caret}
1 |> ignore

()
Copy link
Member

@auduchinok auduchinok Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure it's a correct sequence expression node after the modification, i.e. it should be something like

seqExpr
  1
  1 |> ignore
  ()

and not like

seqExpr
  seqExpr
    1
    1 |> ignore
  ()

It'd be great to have this logic reused in InlineVar refactoring later.

We should probably dump the resulting tree in some of the tests with modifications.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Module

let a =
let b{caret} =
let c = 1
c + c

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module Module

let a =
let {caret}c = 1
c + c |> ignore

()
Copy link
Member

@auduchinok auduchinok Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above:

letExpr
  binding
    expr
  seqExpr
    c + c |> ignore

    ()

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module Module

let a =
let b{caret} =
if true then
1
else
2

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module Module

let a =
{caret}if true then
1
else
2
|> ignore

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Module

let a =
let b{caret} =
match () with
| a -> 1
| b -> 2

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Module

let a =
{caret}match () with
| a -> 1
| b -> 2
|> ignore

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Module

let a =
let b{caret} =
match () with
| a -> 1
| b ->

2

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Module

let a =
{caret}match () with
| a -> 1
| b ->

2
|> ignore

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Module

let a =
let b{caret} =
if true then 1 else

2

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Module

let a =
{caret}if true then 1 else

2
|> ignore

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Module

let a =
let b{caret} = (
let a = 1
let b = 2

a + b
)

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Module

let a =
{caret}(
let a = 1
let b = 2

a + b |> ignore
)

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module Module

let a =
let b{caret} =
let a = 1
let b = 2

()

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Module

let a =
let {caret}a = 1
let b = 2

()

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Module

let a =
let {caret}a =
lazy
1

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Module

let a =
l{caret}azy
1
|> ignore

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Module

let a =
let {caret}a =
function
| b -> 1
| c -> 2

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Module

let a =
{caret}function
| b -> 1
| c -> 2
|> ignore

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Module

let a =
let {caret}a =
fun x ->
x + 1

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Module

let a =
{caret}fun x ->
x + 1
|> ignore

()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Module

let a =
let {caret}a =
try () with
| b -> ()
| c -> ()

()
Loading