Skip to content

Few more IDE scenarios for top-level statements#44455

Merged
jcouv merged 5 commits intodotnet:features/SimpleProgramsfrom
jcouv:simple-ide
May 21, 2020
Merged

Few more IDE scenarios for top-level statements#44455
jcouv merged 5 commits intodotnet:features/SimpleProgramsfrom
jcouv:simple-ide

Conversation

@jcouv
Copy link
Member

@jcouv jcouv commented May 20, 2020

Fixes #44272 (RemoveUnusedLocalFunction)
Fixes #44273 (RemoveUnusedVariable)
Fixes ConvertSwitchStatementToExpression

@jcouv jcouv self-assigned this May 20, 2020
@jcouv jcouv marked this pull request as ready for review May 20, 2020 23:34
@jcouv jcouv requested a review from a team as a code owner May 20, 2020 23:34
@jcouv
Copy link
Member Author

jcouv commented May 20, 2020

Tagging @CyrusNajmabadi @sharwell @AlekseyTs

Comment on lines +84 to +85
var switchNode = switchLocation.FindNode(getInnermostNodeForTie: true, cancellationToken);
var switchStatement = (SwitchStatementSyntax)(switchNode);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var switchNode = switchLocation.FindNode(getInnermostNodeForTie: true, cancellationToken);
var switchStatement = (SwitchStatementSyntax)(switchNode);
var switchStatement = (SwitchStatementSyntax)switchLocation.FindNode(getInnermostNodeForTie: true, cancellationToken);

Comment on lines 103 to 107
if (nextStatement.IsParentKind(SyntaxKind.GlobalStatement))
{
nextStatement = nextStatement.Parent;
}
editor.RemoveNode(nextStatement);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (nextStatement.IsParentKind(SyntaxKind.GlobalStatement))
{
nextStatement = nextStatement.Parent;
}
editor.RemoveNode(nextStatement);
editor.RemoveNode(nextStatement.IsParentKind(SyntaxKind.GlobalStatement) ? nextStatement.Parent : nextStatement);

(can keep teh var above as well).

{
nodeToRemove = localFunction.Parent;
}
editor.RemoveNode(nodeToRemove);
Copy link
Contributor

Choose a reason for hiding this comment

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

same feedback.

also, i'm really hating this pattern :-/

{
editor.RemoveNode(localFunction);
SyntaxNode nodeToRemove = localFunction;
if (localFunction.Parent.Kind() == SyntaxKind.GlobalStatement)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (localFunction.Parent.Kind() == SyntaxKind.GlobalStatement)
if (localFunction.IsParentKind(SyntaxKind.GlobalStatement))

{
node = node.Parent;
}
RemoveNode(editor, node, syntaxFacts);
Copy link
Contributor

Choose a reason for hiding this comment

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

same feedback.

Comment on lines +39 to +49
if (!one.IsParentKind(SyntaxKind.GlobalStatement))
{
return false;
}

if (!one.IsParentKind(SyntaxKind.GlobalStatement))
{
return false;
}

return one.Parent.Parent == other.Parent.Parent;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!one.IsParentKind(SyntaxKind.GlobalStatement))
{
return false;
}
if (!one.IsParentKind(SyntaxKind.GlobalStatement))
{
return false;
}
return one.Parent.Parent == other.Parent.Parent;
return one.IsParentKind(SyntaxKind.GlobalStatement) &&
one.IsParentKind(SyntaxKind.GlobalStatement) &&
one.Parent.Parent == other.Parent.Parent;

Copy link
Contributor

Choose a reason for hiding this comment

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

💭 The last condition is redundant. Global statements are always siblings if they are in the same tree.

Comment on lines +1916 to +1922
await VerifyCS.VerifyCodeFixAsync(source, new[] {
// error CS9004: Program using top-level statements must be an executable.
DiagnosticResult.CompilerError("CS9004"),
// error CS8652: The feature 'top-level statements' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version.
DiagnosticResult.CompilerError("CS8652").WithSpan(2, 1, 2, 11).WithArguments("top-level statements"),
},
fixedSource);
Copy link
Contributor

@sharwell sharwell May 20, 2020

Choose a reason for hiding this comment

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

📝 The test should be run with this feature enabled

await new VerifyCS.Test {
  TestCode = source,
  FixedCode = fixedSource,
  LanguageVersion = LanguageVersion.Preview,
}.RunAsync();

}");
}

[WorkItem(44454, "https://github.com/dotnet/roslyn/issues/44454")]
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 Another pull request is already submitted to fix this

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome. I'll remove this test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants