From 822bcd6b8ebd66e4cacb6d291bd9f4d6e4de52b4 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 4 Jul 2014 15:58:48 +0200 Subject: [PATCH] Notify user of Structural Search Groovy Script errors while searching/inspecting (IDEA-126731) --- .../StructuralSearchException.java | 13 ++ .../impl/matcher/MatcherImpl.java | 8 +- .../matcher/predicates/ScriptPredicate.java | 2 +- .../matcher/predicates/ScriptSupport.java | 10 +- .../highlightTemplate/SSBasedInspection.java | 22 ++- .../replace/impl/ReplacementBuilder.java | 2 +- .../plugin/ui/SearchCommand.java | 153 ++++++++++-------- .../source/messages/SSRBundle.properties | 3 + 8 files changed, 134 insertions(+), 79 deletions(-) create mode 100644 plugins/structuralsearch/source/com/intellij/structuralsearch/StructuralSearchException.java diff --git a/plugins/structuralsearch/source/com/intellij/structuralsearch/StructuralSearchException.java b/plugins/structuralsearch/source/com/intellij/structuralsearch/StructuralSearchException.java new file mode 100644 index 000000000000..f925285054f0 --- /dev/null +++ b/plugins/structuralsearch/source/com/intellij/structuralsearch/StructuralSearchException.java @@ -0,0 +1,13 @@ + +package com.intellij.structuralsearch; + +/** + * @author Bas Leijdekkers + */ +public class StructuralSearchException extends RuntimeException { + public StructuralSearchException() {} + + public StructuralSearchException(String message) { + super(message); + } +} diff --git a/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/MatcherImpl.java b/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/MatcherImpl.java index 1403f3f3c185..d667b068ee48 100644 --- a/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/MatcherImpl.java +++ b/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/MatcherImpl.java @@ -497,7 +497,13 @@ public class MatcherImpl { final Runnable task = tasks.removeFirst(); try { task.run(); - } catch (ProcessCanceledException e) { + } + catch (ProcessCanceledException e) { + ended = true; + clearSchedule(); + throw e; + } + catch (StructuralSearchException e) { ended = true; clearSchedule(); throw e; diff --git a/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/predicates/ScriptPredicate.java b/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/predicates/ScriptPredicate.java index 7e91ed96b18f..9310eb9e70a0 100644 --- a/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/predicates/ScriptPredicate.java +++ b/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/predicates/ScriptPredicate.java @@ -11,7 +11,7 @@ public class ScriptPredicate extends AbstractStringBasedPredicate { public ScriptPredicate(String name, String within) { super(name, within); - scriptSupport = new ScriptSupport(within); + scriptSupport = new ScriptSupport(within, name); } public boolean match(PsiElement node, PsiElement match, int start, int end, MatchContext context) { diff --git a/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/predicates/ScriptSupport.java b/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/predicates/ScriptSupport.java index 580d6582d01e..37575d60b8b4 100644 --- a/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/predicates/ScriptSupport.java +++ b/plugins/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/predicates/ScriptSupport.java @@ -1,10 +1,11 @@ package com.intellij.structuralsearch.impl.matcher.predicates; import com.intellij.openapi.diagnostic.Logger; -import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiIdentifier; import com.intellij.structuralsearch.MatchResult; +import com.intellij.structuralsearch.SSRBundle; +import com.intellij.structuralsearch.StructuralSearchException; import com.intellij.structuralsearch.impl.matcher.MatchResultImpl; import groovy.lang.Binding; import groovy.lang.GroovyRuntimeException; @@ -29,11 +30,11 @@ import java.util.List; public class ScriptSupport { private final Script script; - public ScriptSupport(String text) { + public ScriptSupport(String text, String name) { File scriptFile = new File(text); GroovyShell shell = new GroovyShell(); try { - script = scriptFile.exists() ? shell.parse(scriptFile):shell.parse(text); + script = scriptFile.exists() ? shell.parse(scriptFile):shell.parse(text, name); } catch (Exception ex) { Logger.getInstance(getClass().getName()).error(ex); throw new RuntimeException(ex); @@ -60,8 +61,7 @@ public class ScriptSupport { Object o = script.run(); return String.valueOf(o); } catch (GroovyRuntimeException ex) { - Logger.getInstance(getClass().getName()).error(ex); - return StringUtil.convertLineSeparators(ex.getLocalizedMessage()); + throw new StructuralSearchException(SSRBundle.message("groovy.script.error", ex.getMessage())); } } diff --git a/plugins/structuralsearch/source/com/intellij/structuralsearch/inspection/highlightTemplate/SSBasedInspection.java b/plugins/structuralsearch/source/com/intellij/structuralsearch/inspection/highlightTemplate/SSBasedInspection.java index c08d0d615c1a..7a38ddb2ea65 100644 --- a/plugins/structuralsearch/source/com/intellij/structuralsearch/inspection/highlightTemplate/SSBasedInspection.java +++ b/plugins/structuralsearch/source/com/intellij/structuralsearch/inspection/highlightTemplate/SSBasedInspection.java @@ -18,6 +18,9 @@ package com.intellij.structuralsearch.inspection.highlightTemplate; import com.intellij.codeInsight.FileModificationService; import com.intellij.codeInspection.*; import com.intellij.dupLocator.iterators.CountingNodeIterator; +import com.intellij.notification.Notification; +import com.intellij.notification.NotificationType; +import com.intellij.notification.Notifications; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.InvalidDataException; import com.intellij.openapi.util.Pair; @@ -28,6 +31,7 @@ import com.intellij.psi.PsiElementVisitor; import com.intellij.structuralsearch.MatchResult; import com.intellij.structuralsearch.Matcher; import com.intellij.structuralsearch.SSRBundle; +import com.intellij.structuralsearch.StructuralSearchException; import com.intellij.structuralsearch.impl.matcher.MatchContext; import com.intellij.structuralsearch.impl.matcher.MatcherImpl; import com.intellij.structuralsearch.impl.matcher.filters.LexicalNodesFilter; @@ -46,9 +50,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; import javax.swing.*; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; +import java.util.*; /** * @author cdr @@ -56,12 +58,14 @@ import java.util.List; public class SSBasedInspection extends LocalInspectionTool { static final String SHORT_NAME = "SSBasedInspection"; private List myConfigurations = new ArrayList(); + private Set myProblemsReported = new HashSet(1); public void writeSettings(@NotNull Element node) throws WriteExternalException { ConfigurationManager.writeConfigurations(node, myConfigurations, Collections.emptyList()); } public void readSettings(@NotNull Element node) throws InvalidDataException { + myProblemsReported.clear(); myConfigurations.clear(); ConfigurationManager.readConfigurations(node, myConfigurations, new ArrayList()); } @@ -115,7 +119,17 @@ public class SSBasedInspection extends LocalInspectionTool { if (MatcherImpl.checkIfShouldAttemptToMatch(context, matchedNodes)) { final int nodeCount = context.getPattern().getNodeCount(); - matcher.processMatchesInElement(context, configuration, new CountingNodeIterator(nodeCount, matchedNodes), processor); + try { + matcher.processMatchesInElement(context, configuration, new CountingNodeIterator(nodeCount, matchedNodes), processor); + } + catch (StructuralSearchException e) { + if (myProblemsReported.add(configuration.getName())) { // don't overwhelm the user with messages + Notifications.Bus.notify(new Notification(SSRBundle.message("structural.search.title"), + SSRBundle.message("template.problem", configuration.getName()), + e.getMessage(), + NotificationType.ERROR), element.getProject()); + } + } matchedNodes.reset(); } } diff --git a/plugins/structuralsearch/source/com/intellij/structuralsearch/plugin/replace/impl/ReplacementBuilder.java b/plugins/structuralsearch/source/com/intellij/structuralsearch/plugin/replace/impl/ReplacementBuilder.java index 15fa33959696..33291ecf6aea 100644 --- a/plugins/structuralsearch/source/com/intellij/structuralsearch/plugin/replace/impl/ReplacementBuilder.java +++ b/plugins/structuralsearch/source/com/intellij/structuralsearch/plugin/replace/impl/ReplacementBuilder.java @@ -181,7 +181,7 @@ public final class ReplacementBuilder { if (scriptSupport == null) { String constraint = options.getVariableDefinition(info.getName()).getScriptCodeConstraint(); - scriptSupport = new ScriptSupport(StringUtil.stripQuotesAroundValue(constraint)); + scriptSupport = new ScriptSupport(StringUtil.stripQuotesAroundValue(constraint), info.getName()); replacementVarsMap.put(info.getName(), scriptSupport); } return scriptSupport.evaluate((MatchResultImpl)match, null); diff --git a/plugins/structuralsearch/source/com/intellij/structuralsearch/plugin/ui/SearchCommand.java b/plugins/structuralsearch/source/com/intellij/structuralsearch/plugin/ui/SearchCommand.java index cd5310a1e729..a6591ec29fb3 100644 --- a/plugins/structuralsearch/source/com/intellij/structuralsearch/plugin/ui/SearchCommand.java +++ b/plugins/structuralsearch/source/com/intellij/structuralsearch/plugin/ui/SearchCommand.java @@ -1,22 +1,24 @@ package com.intellij.structuralsearch.plugin.ui; +import com.intellij.notification.NotificationGroup; +import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.project.Project; +import com.intellij.openapi.ui.MessageType; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.openapi.wm.ToolWindowId; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; import com.intellij.psi.PsiNameIdentifierOwner; -import com.intellij.structuralsearch.MatchResult; -import com.intellij.structuralsearch.MatchResultSink; -import com.intellij.structuralsearch.MatchingProcess; -import com.intellij.structuralsearch.SSRBundle; +import com.intellij.structuralsearch.*; import com.intellij.structuralsearch.impl.matcher.MatchResultImpl; import com.intellij.structuralsearch.plugin.StructuralSearchPlugin; import com.intellij.structuralsearch.plugin.ui.actions.DoSearchAction; import com.intellij.usageView.UsageInfo; import com.intellij.usages.Usage; import com.intellij.usages.UsageInfo2UsageAdapter; +import com.intellij.util.Alarm; import com.intellij.util.ObjectUtils; import com.intellij.util.Processor; @@ -40,72 +42,89 @@ public class SearchCommand { public void findUsages(final Processor processor) { final ProgressIndicator progress = ProgressManager.getInstance().getProgressIndicator(); - DoSearchAction.execute( - project, - new MatchResultSink() { - int count; + try { + DoSearchAction.execute( + project, + new MatchResultSink() { + int count; - public void setMatchingProcess(MatchingProcess _process) { - process = _process; - findStarted(); - } - - public void processFile(PsiFile element) { - final VirtualFile virtualFile = element.getVirtualFile(); - if (virtualFile!=null) - progress.setText( SSRBundle.message("looking.in.progress.message",virtualFile.getPresentableName()) ); - } - - public void matchingFinished() { - findEnded(); - progress.setText( SSRBundle.message("found.progress.message", count ) ); - } - - public ProgressIndicator getProgressIndicator() { - return progress; - } - - public void newMatch(MatchResult result) { - UsageInfo info; - - if (MatchResult.MULTI_LINE_MATCH.equals(result.getName())) { - int start = -1; - int end = -1; - PsiElement parent = result.getMatchRef().getElement().getParent(); - - for (final MatchResult matchResult : ((MatchResultImpl)result).getMatches()) { - PsiElement el = matchResult.getMatchRef().getElement(); - final int elementStart = el.getTextRange().getStartOffset(); - - if (start == -1 || start > elementStart) { - start = elementStart; - } - final int newend = elementStart + el.getTextLength(); - - if (newend > end) { - end = newend; - } - } - - final int parentStart = parent.getTextRange().getStartOffset(); - int startOffset = start - parentStart; - info = new UsageInfo(parent,startOffset,end - parentStart); - } else { - PsiElement element = result.getMatch(); - if (element instanceof PsiNameIdentifierOwner) { - element = ObjectUtils.notNull(((PsiNameIdentifierOwner)element).getNameIdentifier(), element); - } - info = new UsageInfo(element, result.getStart(), result.getEnd() == -1 ? element.getTextLength() : result.getEnd()); + public void setMatchingProcess(MatchingProcess _process) { + process = _process; + findStarted(); } - Usage usage = new UsageInfo2UsageAdapter(info); - processor.process(usage); - foundUsage(result, usage); - ++count; - } - }, - context.getConfiguration() - ); + public void processFile(PsiFile element) { + final VirtualFile virtualFile = element.getVirtualFile(); + if (virtualFile != null) + progress.setText(SSRBundle.message("looking.in.progress.message", virtualFile.getPresentableName())); + } + + public void matchingFinished() { + new Throwable().printStackTrace(System.out); + findEnded(); + progress.setText(SSRBundle.message("found.progress.message", count)); + } + + public ProgressIndicator getProgressIndicator() { + return progress; + } + + public void newMatch(MatchResult result) { + UsageInfo info; + + if (MatchResult.MULTI_LINE_MATCH.equals(result.getName())) { + int start = -1; + int end = -1; + PsiElement parent = result.getMatchRef().getElement().getParent(); + + for (final MatchResult matchResult : ((MatchResultImpl)result).getMatches()) { + PsiElement el = matchResult.getMatchRef().getElement(); + final int elementStart = el.getTextRange().getStartOffset(); + + if (start == -1 || start > elementStart) { + start = elementStart; + } + final int newend = elementStart + el.getTextLength(); + + if (newend > end) { + end = newend; + } + } + + final int parentStart = parent.getTextRange().getStartOffset(); + int startOffset = start - parentStart; + info = new UsageInfo(parent, startOffset, end - parentStart); + } + else { + PsiElement element = result.getMatch(); + if (element instanceof PsiNameIdentifierOwner) { + element = ObjectUtils.notNull(((PsiNameIdentifierOwner)element).getNameIdentifier(), element); + } + info = new UsageInfo(element, result.getStart(), result.getEnd() == -1 ? element.getTextLength() : result.getEnd()); + } + + Usage usage = new UsageInfo2UsageAdapter(info); + processor.process(usage); + foundUsage(result, usage); + ++count; + } + }, + context.getConfiguration() + ); + } + catch (final StructuralSearchException e) { + final Alarm alarm = new Alarm(); + alarm.addRequest( + new Runnable() { + @Override + public void run() { + NotificationGroup.toolWindowGroup("Structural Search", ToolWindowId.FIND, true) + .createNotification(SSRBundle.message("problem", e.getMessage()), MessageType.ERROR).notify(project); + } + }, + 100, ModalityState.NON_MODAL + ); + } } public void stopAsyncSearch() { diff --git a/plugins/structuralsearch/source/messages/SSRBundle.properties b/plugins/structuralsearch/source/messages/SSRBundle.properties index 867e3fe74c2f..488c96e84435 100644 --- a/plugins/structuralsearch/source/messages/SSRBundle.properties +++ b/plugins/structuralsearch/source/messages/SSRBundle.properties @@ -225,6 +225,9 @@ predefined.configuration.logging.without.if=logging without if predefined.configuration.class.with.parameterless.constructors=classes with parameterless constructors predefined.configuration.static.fields.without.final=static fields that are not final invalid.groovy.script=Invalid Groovy Script +groovy.script.error=Groovy Script execution error: {0} +template.problem=Structural Search Inspection problem in template ''{0}'' +problem=Structural Search problem: {0} complete.match.variable.name=Complete Match predefined.configuration.sample.method.invokation.with.constant.argument=sample method invocation with constant parameter predefined.configuration.interfaces.having.no.descendants=interface that is not implemented or extended