Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.ArgumentNullException in SyntaxNodeExtensions.RemoveNode #4412

Open
default0 opened this issue Aug 7, 2015 · 4 comments
Open

System.ArgumentNullException in SyntaxNodeExtensions.RemoveNode #4412

default0 opened this issue Aug 7, 2015 · 4 comments
Labels
Area-Compilers Bug Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Milestone

Comments

@default0
Copy link

default0 commented Aug 7, 2015

The following Code causes an exception:

void M()
{
    ExpressionStatementSyntax expressionStatement = SyntaxFactory.ExpressionStatement(SyntaxFactory.ParseExpression("a + b"));
    expressionStatement.RemoveNode(expressionStatement.Expression, SyntaxRemoveOptions.KeepNoTrivia);
}

My assumption is that an AST with an ExpressionStatementSyntax that has no ExpressionSyntax is invalid, and thus an error occurs. The error message could be a lot more descriptive about that, though...

Exception:

   at Microsoft.CodeAnalysis.CSharp.SyntaxFactory.ExpressionStatement(ExpressionSyntax expression, SyntaxToken semicolonToken)
   at Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionStatementSyntax.Update(ExpressionSyntax expression, SyntaxToken semicolonToken)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitExpressionStatement(ExpressionStatementSyntax node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionStatementSyntax.Accept[TResult](CSharpSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxNodeRemover.SyntaxRemover.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitListElement[TNode](TNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitList[TNode](SyntaxList`1 list)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitBlock(BlockSyntax node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.BlockSyntax.Accept[TResult](CSharpSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxNodeRemover.SyntaxRemover.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitMethodDeclaration(MethodDeclarationSyntax node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.MethodDeclarationSyntax.Accept[TResult](CSharpSyntaxVisitor`1 visitor)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxNodeRemover.SyntaxRemover.Visit(SyntaxNode node)
   at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxNodeRemover.RemoveNodes[TRoot](TRoot root, IEnumerable`1 nodes, SyntaxRemoveOptions options)
   at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxNode.RemoveNodesCore(IEnumerable`1 nodes, SyntaxRemoveOptions options)
   at Microsoft.CodeAnalysis.SyntaxNodeExtensions.RemoveNode[TRoot](TRoot root, SyntaxNode node, SyntaxRemoveOptions options)
   at RoslynBugTest.Program.Main() in c:\Users\cs\Documents\Visual Studio 2013\Projects\RoslynBugTest\RoslynBugTest\Program.cs:Zeile 31.
   at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
   at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
   at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()
@gafter gafter added this to the Unknown milestone Aug 7, 2015
@jaredpar jaredpar added Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. Bug help wanted The issue is "up for grabs" - add a comment if you are interested in working on it labels Dec 7, 2015
@vamuzumd
Copy link

vamuzumd commented Sep 4, 2020

I am facing the same issue when trying to remove 'Message' IdentifierNode from the ex?.GetBaseException.Message expression.

The stack trace is as follows:-

'parent.RemoveNode(messageNode, SyntaxRemoveOptions.KeepExteriorTrivia)' threw an exception of type 'System.ArgumentNullException'

at Microsoft.CodeAnalysis.CSharp.SyntaxFactory.MemberAccessExpression(SyntaxKind kind, ExpressionSyntax expression, SyntaxToken operatorToken, SimpleNameSyntax name)\r\n at Microsoft.CodeAnalysis.CSharp.Syntax.MemberAccessExpressionSyntax.Update(ExpressionSyntax expression, SyntaxToken operatorToken, SimpleNameSyntax name)\r\n at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.VisitMemberAccessExpression(MemberAccessExpressionSyntax node)\r\n at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxRewriter.Visit(SyntaxNode node)\r\n at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxNodeRemover.SyntaxRemover.Visit(SyntaxNode node)\r\n at Microsoft.CodeAnalysis.CSharp.Syntax.SyntaxNodeRemover.RemoveNodes[TRoot](TRoot root, IEnumerable1 nodes, SyntaxRemoveOptions options)\r\n at Microsoft.CodeAnalysis.CSharp.CSharpSyntaxNode.RemoveNodesCore(IEnumerable1 nodes, SyntaxRemoveOptions options)\r\n at Microsoft.CodeAnalysis.SyntaxNodeExtensions.RemoveNode[TRoot](TRoot root, SyntaxNode node, SyntaxRemoveOptions options)"

Any ETA on when this issue would be fixed?

@CyrusNajmabadi
Copy link
Member

Any ETA on when this issue would be fixed?

There is nothing to "fix" here, outside of throwing a more clear exception. What you are trying to do is illegal. A member-access-expression (i.e. expr.b) is required to have a non-null left and non-null right. You cannot remove either side without causing an invariant exception (which manifests here as an argumentnullexception).

If you want to convert the above to ex?.GetbaseException., then right thing to do is to not RemoveNode on 'Message', but instead do a ReplaceNode of Message with an empty IdentifierSyntax.

You can get a sense for what tree you need to convert the 'before' into by using a quoter tool like https://roslynquoter.azurewebsites.net/ . This will show you what the expected tree form is for something like ex?.GetbaseException.. You will have to generate that form, otherwise the behavior of all further parts of the system is undefined.

@windhandel
Copy link

Hi @CyrusNajmabadi I think the real bug (similar API, not identical) is that you can't tell based on the exception whether it's the first or second parameter that's the cause of the exception.

For example:

documentEditor.InsertBefore(firstMemberOfClass, declarationExpression);

@CyrusNajmabadi
Copy link
Member

@windhandel We'd welcome a PR that helped with that. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. help wanted The issue is "up for grabs" - add a comment if you are interested in working on it
Projects
None yet
Development

No branches or pull requests

7 participants