eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
157 stars 125 forks source link

[formatter] IndexOutOfBoundsException in applying formatter to code having errors #1437

Open sagarrohat opened 12 months ago

sagarrohat commented 12 months ago

Relates to: https://bugs.eclipse.org/bugs/show_bug.cgi?id=471825 https://github.com/vi-eclipse/Eclipse-JDT/issues/10

How to reproduce the bug:

The code given in this comment: https://bugs.eclipse.org/bugs/show_bug.cgi?id=471825#c11

or

package testproject;

public class Application extends B {
protected int f = 11;

As far I can understand, it fails because we ignore errors if the kind is K_COMPILATION_UNIT: https://github.com/eclipse-jdt/eclipse.jdt.core/blob/master/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/DefaultCodeFormatter.java#L317 and it proceeds to find the closing bracket.

For me, if a code has errors then we should not format it at all but I can't find any comment on why ignoreErrors was set to true for K_COMPILATION_UNIT in the original code which was introduced in this commit: https://github.com/eclipse-jdt/eclipse.jdt.core/commit/b0e753009ffb9bf0d92f75f621fb0d6188adb57e

One possible fix could be to not ignore errors: https://github.com/eclipse-jdt/eclipse.jdt.core/pull/1434

But it fails tests and before fixing those tests I would like to confirm this is the expected behavior.

mateusz-matela commented 8 months ago

The idea was that the formatter would only skip AST nodes marked as malformed (see preVisit2() in all of the formatter's ASTVisitors). This in theory should be enough to avoid this kind of problems, while letting us reformat at least fragments that are syntactically correct. So maybe a good solution with this example would be if the parser/compiler marked the whole TypeDeclaration as malformed? I've already suggested that in https://bugs.eclipse.org/bugs/show_bug.cgi?id=471825#c38