IDEA-25753 Surround With try-catch etc. - doesn't indent comment correctly

1. Java 'surround with' processing is corrected for the first comment statement;
2. Corresponding tests are added;
3. Green cody policy is applied;
This commit is contained in:
Denis Zhdanov
2011-05-03 17:20:13 +04:00
parent fec8b2b9a2
commit 06b56cf449
33 changed files with 436 additions and 15 deletions

View File

@@ -45,10 +45,15 @@ class JavaWithBlockSurrounder extends JavaStatementsSurrounder{
blockStatement = (PsiBlockStatement)container.addBefore(blockStatement, statements[0]);
PsiCodeBlock body = blockStatement.getCodeBlock();
SurroundWithUtil.indentCommentIfNecessary(body, statements, factory);
body.addRange(statements[0], statements[statements.length - 1]);
container.deleteChildRange(statements[0], statements[statements.length - 1]);
TextRange range = blockStatement.getFirstChild().getTextRange();
PsiElement firstChild = blockStatement.getFirstChild();
if (firstChild == null) {
return null;
}
TextRange range = firstChild.getTextRange();
return new TextRange(range.getEndOffset(), range.getEndOffset());
}
}

View File

@@ -46,10 +46,16 @@ class JavaWithDoWhileSurrounder extends JavaStatementsSurrounder{
doWhileStatement = (PsiDoWhileStatement)container.addAfter(doWhileStatement, statements[statements.length - 1]);
PsiCodeBlock bodyBlock = ((PsiBlockStatement)doWhileStatement.getBody()).getCodeBlock();
PsiStatement body = doWhileStatement.getBody();
if (!(body instanceof PsiBlockStatement)) {
return null;
}
PsiCodeBlock bodyBlock = ((PsiBlockStatement)body).getCodeBlock();
SurroundWithUtil.indentCommentIfNecessary(bodyBlock, statements, factory);
bodyBlock.addRange(statements[0], statements[statements.length - 1]);
container.deleteChildRange(statements[0], statements[statements.length - 1]);
return doWhileStatement.getCondition().getTextRange();
PsiExpression condition = doWhileStatement.getCondition();
return condition == null ? null : condition.getTextRange();
}
}

View File

@@ -47,13 +47,27 @@ class JavaWithForSurrounder extends JavaStatementsSurrounder{
forStatement = (PsiForStatement)container.addAfter(forStatement, statements[statements.length - 1]);
PsiCodeBlock bodyBlock = ((PsiBlockStatement)forStatement.getBody()).getCodeBlock();
PsiStatement body = forStatement.getBody();
if (!(body instanceof PsiBlockStatement)) {
return null;
}
PsiCodeBlock bodyBlock = ((PsiBlockStatement)body).getCodeBlock();
SurroundWithUtil.indentCommentIfNecessary(bodyBlock, statements, factory);
bodyBlock.addRange(statements[0], statements[statements.length - 1]);
container.deleteChildRange(statements[0], statements[statements.length - 1]);
forStatement = CodeInsightUtilBase.forcePsiPostprocessAndRestoreElement(forStatement);
TextRange range1 = forStatement.getInitialization().getTextRange();
TextRange range3 = forStatement.getUpdate().getTextRange();
PsiStatement initialization = forStatement.getInitialization();
if (initialization == null) {
return null;
}
TextRange range1 = initialization.getTextRange();
PsiStatement update = forStatement.getUpdate();
if (update == null) {
return null;
}
TextRange range3 = update.getTextRange();
editor.getDocument().deleteString(range1.getStartOffset(), range3.getEndOffset());
return new TextRange(range1.getStartOffset(), range1.getStartOffset());
}

View File

@@ -47,11 +47,20 @@ class JavaWithIfElseSurrounder extends JavaStatementsSurrounder{
ifStatement = (PsiIfStatement)container.addAfter(ifStatement, statements[statements.length - 1]);
PsiCodeBlock thenBlock = ((PsiBlockStatement)ifStatement.getThenBranch()).getCodeBlock();
PsiStatement thenBranch = ifStatement.getThenBranch();
if (!(thenBranch instanceof PsiBlockStatement)) {
return null;
}
PsiCodeBlock thenBlock = ((PsiBlockStatement)thenBranch).getCodeBlock();
SurroundWithUtil.indentCommentIfNecessary(thenBlock, statements, factory);
thenBlock.addRange(statements[0], statements[statements.length - 1]);
container.deleteChildRange(statements[0], statements[statements.length - 1]);
ifStatement = CodeInsightUtilBase.forcePsiPostprocessAndRestoreElement(ifStatement);
TextRange range = ifStatement.getCondition().getTextRange();
PsiExpression condition = ifStatement.getCondition();
if (condition == null) {
return null;
}
TextRange range = condition.getTextRange();
editor.getDocument().deleteString(range.getStartOffset(), range.getEndOffset());
return new TextRange(range.getStartOffset(), range.getStartOffset());
}

View File

@@ -50,6 +50,7 @@ public class JavaWithIfSurrounder extends JavaStatementsSurrounder{
final PsiStatement thenBranch = ifStatement.getThenBranch();
if (thenBranch != null) {
PsiCodeBlock thenBlock = ((PsiBlockStatement)thenBranch).getCodeBlock();
SurroundWithUtil.indentCommentIfNecessary(thenBlock, statements, factory);
thenBlock.addRange(statements[0], statements[statements.length - 1]);
container.deleteChildRange(statements[0], statements[statements.length - 1]);
}

View File

@@ -48,11 +48,19 @@ class JavaWithSynchronizedSurrounder extends JavaStatementsSurrounder{
synchronizedStatement = (PsiSynchronizedStatement)container.addAfter(synchronizedStatement, statements[statements.length - 1]);
PsiCodeBlock synchronizedBlock = synchronizedStatement.getBody();
if (synchronizedBlock == null) {
return null;
}
SurroundWithUtil.indentCommentIfNecessary(synchronizedBlock, statements, factory);
synchronizedBlock.addRange(statements[0], statements[statements.length - 1]);
container.deleteChildRange(statements[0], statements[statements.length - 1]);
synchronizedStatement = CodeInsightUtilBase.forcePsiPostprocessAndRestoreElement(synchronizedStatement);
TextRange range = synchronizedStatement.getLockExpression().getTextRange();
PsiExpression lockExpression = synchronizedStatement.getLockExpression();
if (lockExpression == null) {
return null;
}
TextRange range = lockExpression.getTextRange();
editor.getDocument().deleteString(range.getStartOffset(), range.getEndOffset());
return new TextRange(range.getStartOffset(), range.getStartOffset());
}

View File

@@ -72,6 +72,7 @@ public class JavaWithTryCatchSurrounder extends JavaStatementsSurrounder {
tryStatement = (PsiTryStatement)container.addAfter(tryStatement, statements[statements.length - 1]);
PsiCodeBlock tryBlock = tryStatement.getTryBlock();
SurroundWithUtil.indentCommentIfNecessary(tryBlock, statements, factory);
tryBlock.addRange(statements[0], statements[statements.length - 1]);
PsiCatchSection[] catchSections = tryStatement.getCatchSections();

View File

@@ -47,10 +47,17 @@ class JavaWithTryFinallySurrounder extends JavaStatementsSurrounder{
tryStatement = (PsiTryStatement)container.addAfter(tryStatement, statements[statements.length - 1]);
PsiCodeBlock tryBlock = tryStatement.getTryBlock();
if (tryBlock == null) {
return null;
}
SurroundWithUtil.indentCommentIfNecessary(tryBlock, statements, factory);
tryBlock.addRange(statements[0], statements[statements.length - 1]);
container.deleteChildRange(statements[0], statements[statements.length - 1]);
PsiCodeBlock finallyBlock = tryStatement.getFinallyBlock();
if (finallyBlock == null) {
return null;
}
int offset = finallyBlock.getTextRange().getStartOffset() + 1;
return new TextRange(offset, offset);
}

View File

@@ -46,10 +46,16 @@ class JavaWithWhileSurrounder extends JavaStatementsSurrounder{
whileStatement = (PsiWhileStatement)container.addAfter(whileStatement, statements[statements.length - 1]);
PsiCodeBlock bodyBlock = ((PsiBlockStatement)whileStatement.getBody()).getCodeBlock();
PsiStatement body = whileStatement.getBody();
if (!(body instanceof PsiBlockStatement)) {
return null;
}
PsiCodeBlock bodyBlock = ((PsiBlockStatement)body).getCodeBlock();
SurroundWithUtil.indentCommentIfNecessary(bodyBlock, statements, factory);
bodyBlock.addRange(statements[0], statements[statements.length - 1]);
container.deleteChildRange(statements[0], statements[statements.length - 1]);
return whileStatement.getCondition().getTextRange();
PsiExpression condition = whileStatement.getCondition();
return condition == null ? null : condition.getTextRange();
}
}

View File

@@ -16,20 +16,29 @@
*/
package com.intellij.codeInsight.generation.surroundWith;
import com.intellij.lang.ASTNode;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.util.TextRange;
import com.intellij.psi.*;
import com.intellij.psi.codeStyle.CodeStyleManager;
import com.intellij.psi.impl.source.tree.ElementType;
import com.intellij.psi.search.GlobalSearchScope;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.util.PsiTypesUtil;
import com.intellij.psi.util.PsiUtilBase;
import com.intellij.util.IncorrectOperationException;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
public class SurroundWithUtil {
private static final Logger LOG = Logger.getInstance("#com.intellij.codeInsight.generation.surroundWith.SurroundWithUtil");
private SurroundWithUtil() {
}
static PsiElement[] moveDeclarationsOut(PsiElement block, PsiElement[] statements, boolean generateInitializers) {
try{
PsiManager psiManager = block.getManager();
@@ -38,7 +47,7 @@ public class SurroundWithUtil {
for (PsiElement statement : statements) {
if (statement instanceof PsiDeclarationStatement) {
PsiDeclarationStatement declaration = (PsiDeclarationStatement)statement;
if (needToDeclareOut(block, statements, declaration)) {
if (needToDeclareOut(statements, declaration)) {
PsiElement[] elements = declaration.getDeclaredElements();
for (PsiElement element : elements) {
PsiVariable var = (PsiVariable)element;
@@ -90,7 +99,7 @@ public class SurroundWithUtil {
}
}
private static boolean needToDeclareOut(PsiElement block, PsiElement[] statements, PsiDeclarationStatement statement) {
private static boolean needToDeclareOut(PsiElement[] statements, PsiDeclarationStatement statement) {
PsiElement[] elements = statement.getDeclaredElements();
PsiElement lastStatement = statements[statements.length - 1];
int endOffset = lastStatement.getTextRange().getEndOffset();
@@ -109,7 +118,7 @@ public class SurroundWithUtil {
return false;
}
public static TextRange getRangeToSelect (PsiCodeBlock block) {
public static TextRange getRangeToSelect (@NotNull PsiCodeBlock block) {
PsiElement first = block.getFirstBodyElement();
if (first instanceof PsiWhiteSpace) {
first = first.getNextSibling();
@@ -118,10 +127,111 @@ public class SurroundWithUtil {
int offset = block.getTextRange().getStartOffset() + 1;
return new TextRange(offset, offset);
}
PsiElement last = block.getRBrace().getPrevSibling();
PsiJavaToken rBrace = block.getRBrace();
PsiElement last = rBrace.getPrevSibling();
if (last instanceof PsiWhiteSpace) {
last = last.getPrevSibling();
}
return new TextRange(first.getTextRange().getStartOffset(), last.getTextRange().getEndOffset());
}
/**
* Performs indentation (if necessary) of the given code block that surrounds target statements.
* <p/>
* The main trick here is to handle situations like the one below:
* <pre>
* void test() {
* // This is comment
* int i = 1;
* }
* </pre>
* The problem is that surround block doesn't contain any indent spaces, hence, the first statement is inserted to the
* zero column. But we have a dedicated code style setting <code>'keep comment at first column'</code>, i.e. the comment
* will not be moved if that setting is checked.
* <p/>
* Current method handles that situation.
*
* @param container code block that surrounds target statements
* @param statements target statements being surrounded
* @param factory factory to use for the new white space element construction
*/
public static void indentCommentIfNecessary(@NotNull PsiCodeBlock container, @Nullable PsiElement[] statements,
@NotNull PsiElementFactory factory)
{
if (statements == null || statements.length <= 0) {
return;
}
PsiElement first = statements[0];
ASTNode node = first.getNode();
if (node == null || !ElementType.JAVA_COMMENT_BIT_SET.contains(node.getElementType())) {
return;
}
ASTNode commentWsText = node.getTreePrev();
if (commentWsText == null || !ElementType.WHITE_SPACE_BIT_SET.contains(commentWsText.getElementType())) {
return;
}
int indent = 0;
CharSequence text = commentWsText.getChars();
for (int i = text.length() - 1; i >= 0; i--, indent++) {
if (text.charAt(i) == '\n') {
break;
}
}
if (indent <= 0) {
return;
}
PsiElement codeBlockWsElement = null;
ASTNode codeBlockWsNode = null;
boolean lbraceFound = false;
for (PsiElement codeBlockChild = container.getFirstChild(); codeBlockChild != null; codeBlockChild = codeBlockChild.getNextSibling()) {
ASTNode childNode = codeBlockChild.getNode();
if (childNode == null) {
continue;
}
if (!lbraceFound) {
if (JavaTokenType.LBRACE == childNode.getElementType()) {
lbraceFound = true;
}
continue;
}
if (ElementType.WHITE_SPACE_BIT_SET.contains(childNode.getElementType())) {
codeBlockWsElement = codeBlockChild;
codeBlockWsNode = childNode;
break;
}
else if (JavaTokenType.RBRACE == childNode.getElementType()) {
break;
}
}
if (codeBlockWsElement != null) {
CharSequence existingWhiteSpaceText = codeBlockWsNode.getChars();
int existingWhiteSpaceEndOffset = existingWhiteSpaceText.length();
for (int i = existingWhiteSpaceEndOffset - 1; i >= 0; i--) {
if (existingWhiteSpaceText.charAt(i) == '\n') {
existingWhiteSpaceEndOffset = i;
break;
}
}
String newWsText = text.subSequence(text.length() - indent, text.length()).toString();
// Add white spaces from all lines except the last one.
if (existingWhiteSpaceEndOffset < existingWhiteSpaceText.length()) {
newWsText = existingWhiteSpaceText.subSequence(0, existingWhiteSpaceEndOffset + 1).toString() + newWsText;
}
PsiElement indentElement = factory.createWhiteSpaceFromText(newWsText);
codeBlockWsElement.replace(indentElement);
}
else {
PsiElement indentElement = factory.createWhiteSpaceFromText(text.subSequence(text.length() - indent, text.length()).toString());
container.add(indentElement);
}
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,9 @@
class Test {
void foo() {
{
// This is comment"
int i = 1;
}<caret>
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,8 @@
class Test {
void foo() {
do {
// This is comment"
int i = 1;
} while (<caret><selection>true</selection>);
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,8 @@
class Test {
void foo() {
for (<caret>) {
// This is comment"
int i = 1;
}
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,9 @@
class Test {
void foo() {
if (<caret>) {
// This is comment"
int i = 1;
} else {
}
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,8 @@
class Test {
void foo() {
if (<caret>) {
// This is comment"
int i = 1;
}
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,10 @@
class Test {
void foo() {
Runnable <caret><selection>runnable</selection> = new Runnable() {
public void run() {
// This is comment"
int i = 1;
}
};
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,8 @@
class Test {
void foo() {
synchronized (<caret>) {
// This is comment"
int i = 1;
}
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,11 @@
class Test {
void foo() {
try {
// This is comment"
int i = 1;
} catch (Exception e) {
<caret><selection>e.printStackTrace(); //To change body of catch statement use File | Settings | File Templates.</selection>
} finally {
}
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,10 @@
class Test {
void foo() {
try {
// This is comment"
int i = 1;
} catch (Exception e) {
<caret><selection>e.printStackTrace(); //To change body of catch statement use File | Settings | File Templates.</selection>
}
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,9 @@
class Test {
void foo() {
try {
// This is comment"
int i = 1;
} finally {<caret>
}
}
}

View File

@@ -0,0 +1,6 @@
class Test {
void foo() {
<selection>// This is comment"
int i = 1;</selection>
}
}

View File

@@ -0,0 +1,8 @@
class Test {
void foo() {
while (<caret><selection>true</selection>) {
// This is comment"
int i = 1;
}
}
}

View File

@@ -0,0 +1,90 @@
/*
* Copyright 2000-2011 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.intellij.codeInsight.generation.surroundWith;
import com.intellij.lang.surroundWith.Surrounder;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.testFramework.LightCodeInsightTestCase;
import org.jetbrains.annotations.NotNull;
/**
* @author Denis Zhdanov
* @since 5/3/11 2:35 PM
*/
public class JavaSurroundWithTest extends LightCodeInsightTestCase {
private static final String BASE_PATH = "/codeInsight/generation/surroundWith/java/";
@SuppressWarnings({"UnusedDeclaration"})
private enum SurroundType {
IF(new JavaWithIfSurrounder()), IF_ELSE(new JavaWithIfElseSurrounder()),
WHILE(new JavaWithWhileSurrounder()), DO_WHILE(new JavaWithDoWhileSurrounder()),
FOR(new JavaWithForSurrounder()),
TRY_CATCH(new JavaWithTryCatchSurrounder()), TRY_FINALLY(new JavaWithTryFinallySurrounder()),
TRY_CATCH_FINALLY(new JavaWithTryCatchFinallySurrounder()),
SYNCHRONIZED(new JavaWithSynchronizedSurrounder()),
RUNNABLE(new JavaWithRunnableSurrounder()),
CODE_BLOCK(new JavaWithBlockSurrounder());
private final Surrounder mySurrounder;
SurroundType(Surrounder surrounder) {
mySurrounder = surrounder;
}
public Surrounder getSurrounder() {
return mySurrounder;
}
public String toFileName() {
StringBuilder result = new StringBuilder();
boolean capitalize = true;
for (char c : toString().toCharArray()) {
if (c == '_') {
capitalize = true;
continue;
}
if (capitalize) {
result.append(Character.toUpperCase(c));
capitalize = false;
}
else {
result.append(Character.toLowerCase(c));
}
}
return result.toString();
}
}
public void testCommentAsFirstSurroundStatement() throws Exception {
String template = "CommentAsFirst%sSurroundStatement";
for (SurroundType type : SurroundType.values()) {
doTest(type, String.format(template, StringUtil.capitalize(type.toFileName())));
}
}
private void doTest(@NotNull SurroundType surroundType, @NotNull String fileName) throws Exception {
configureByFile(BASE_PATH + fileName + ".java");
SurroundWithHandler.invoke(getProject(), getEditor(), getFile(), surroundType.getSurrounder());
checkResultByFile(BASE_PATH + fileName + "_after.java");
}
}