Bug 270148 - ASTParser cannot parse K_CLASS_BODY_DECLARATIONS with syntax errors
Summary: ASTParser cannot parse K_CLASS_BODY_DECLARATIONS with syntax errors
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 M7   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-26 12:54 EDT by Markus Keller CLA
Modified: 2009-06-12 07:51 EDT (History)
5 users (show)

See Also:


Attachments
Proposed fix (28.38 KB, patch)
2009-03-30 08:48 EDT, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2009-03-26 12:54:59 EDT
I20090325-1135

An ASTParser for kind K_CLASS_BODY_DECLARATIONS cannot parse a method declaration with syntax errors, although setStatementsRecovery(true) has been called. When I copy the method foo() below to the clipboard and then paste it to the Package Explorer, this doesn't create a snippet:

	void foo() {
		Integer I = new ${cursor}
	}

But when I add a dummy class declaration around it, then an ASTParser for K_COMPILATION_UNIT can create an AST:

public class Try {
	void foo() {
		Integer I = new ${cursor}
	}
}

I would expect the K_CLASS_BODY_DECLARATIONS to use the same error recovery.
Comment 1 Olivier Thomann CLA 2009-03-26 12:56:45 EDT
I would also expect the same error recovery.
Comment 2 Olivier Thomann CLA 2009-03-27 08:58:17 EDT
Not only the recovery doesn't work, but it returns a compilation unit node when the documentation says that only a type declaration can be returned.
So both issues must be fixed.
Comment 3 Olivier Thomann CLA 2009-03-30 08:48:20 EDT
Created attachment 130235 [details]
Proposed fix
Comment 4 Olivier Thomann CLA 2009-03-31 10:21:10 EDT
Returning a compilation unit is fine in case the source contains errors and the recovery is not enabled.
I'll update the patch accordingly.
Comment 5 Olivier Thomann CLA 2009-03-31 11:08:22 EDT
Released for 3.5M7.
Added regression tests in:
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0702
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0703
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0704
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0705
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0706
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0707
org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2#test0708

I forgot to generate a new patch before I committed the code.
Comment 6 Jay Arthanareeswaran CLA 2009-04-29 01:19:31 EDT
Verified for 3.5M7 using build I20090426-2000
Comment 7 Frederic Fusier CLA 2009-06-12 04:52:03 EDT
Note that while testing changes in the code formatter, I saw some strange behavior around this fix...

Consider the following snippet:

  class MyCommand extends CommandBase
  {
    protected Command subcommand;

    //...

    public void execute()
    {
      // ...
      Compound subcommands = new CompoundCommand();
      subcommands.appendAndExecute(new AddCommand(...));
      if (condition) subcommands.appendAndExecute(new AddCommand(...));
      subcommand = subcommands.unwrap();
    }

    public void undo()
    {
      // ...
      subcommand.undo();
    }

    public void redo()
    {
      // ...
      subcommand.redo();
    }

    public void dispose()
    {
      // ...
      if (subcommand != null)
     {
        subcommand.dispose();
      }
    }
  }

Before the fix was applied, parsing the snippet using the Parser.parseClassBodyDeclarations(...) method returned null. After the fix (e.g. using HEAD), it returns an array with one TypeDeclaration and it seems to be the expected result.

However, considering another snippet:

  class MyCommand extends CompoundCommand
  {
    public void execute()
    {
      // ...
      appendAndExecute(new AddCommand(...));
      if (condition) appendAndExecute(new AddCommand(...));
    }
  }

parsing it with the same method still returns null. After the fix was applied, I would expect a non null result for both snippets... Did I miss something?
Comment 8 Markus Keller CLA 2009-06-12 06:47:40 EDT
(In reply to comment #7)
Please file a new bug for that problem.
Comment 9 Frederic Fusier CLA 2009-06-12 07:51:30 EDT
(In reply to comment #8)
> (In reply to comment #7)
> Please file a new bug for that problem.
> 
I opened bug 280063 for it...