Skip to content
This repository has been archived by the owner on Apr 6, 2022. It is now read-only.

Commit

Permalink
Improved fix for [JENKINS-15526][FIXED JENKINS-16107].
Browse files Browse the repository at this point in the history
  • Loading branch information
uhafner committed Jan 31, 2013
1 parent 390bb4f commit 2987adb
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 11 deletions.
32 changes: 24 additions & 8 deletions src/main/java/hudson/plugins/warnings/GroovyParser.java
Expand Up @@ -10,11 +10,13 @@
import hudson.plugins.warnings.parser.DynamicParser; import hudson.plugins.warnings.parser.DynamicParser;
import hudson.plugins.warnings.parser.Warning; import hudson.plugins.warnings.parser.Warning;
import hudson.util.FormValidation; import hudson.util.FormValidation;
import hudson.util.FormValidation.Kind;


import java.util.Locale; import java.util.Locale;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException; import java.util.regex.PatternSyntaxException;

import javax.annotation.CheckForNull; import javax.annotation.CheckForNull;


import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.StringUtils;
Expand All @@ -38,7 +40,8 @@ public class GroovyParser extends AbstractDescribableImpl<GroovyParser> {
private String linkName; private String linkName;
/** Trend report name. @since 4.0 */ /** Trend report name. @since 4.0 */
private String trendName; private String trendName;
private transient @CheckForNull AbstractWarningsParser parser; @CheckForNull
private transient AbstractWarningsParser parser;


/** /**
* Creates a new instance of {@link GroovyParser}. * Creates a new instance of {@link GroovyParser}.
Expand Down Expand Up @@ -107,6 +110,10 @@ private Object readResolve() {
* otherwise * otherwise
*/ */
public boolean isValid() { public boolean isValid() {
return parser != null;
}

private boolean canCreateParser() {
DescriptorImpl d = new DescriptorImpl(); DescriptorImpl d = new DescriptorImpl();
return d.doCheckScript(script).kind == FormValidation.Kind.OK return d.doCheckScript(script).kind == FormValidation.Kind.OK
&& d.doCheckRegexp(regexp).kind == FormValidation.Kind.OK && d.doCheckRegexp(regexp).kind == FormValidation.Kind.OK
Expand Down Expand Up @@ -174,27 +181,36 @@ public String getExample() {
* @return <code>true</code> if the parser can scan messages spanning * @return <code>true</code> if the parser can scan messages spanning
* multiple lines * multiple lines
*/ */
public boolean hasMultiLineSupport() { public final boolean hasMultiLineSupport() {
return containsNewline(regexp); return containsNewline(regexp);
} }


private static boolean containsNewline(final String expression) { private static boolean containsNewline(final String expression) {
return StringUtils.contains(expression, "\\n"); return StringUtils.contains(expression, "\\n");
} }


private @CheckForNull AbstractWarningsParser createParser() { @CheckForNull
if (isValid()) { private AbstractWarningsParser createParser() {
if (canCreateParser()) {
if (hasMultiLineSupport()) { if (hasMultiLineSupport()) {
return new DynamicDocumentParser(name, regexp, script, linkName, trendName); return new DynamicDocumentParser(name, regexp, script, linkName, trendName);
} else { }
else {
return new DynamicParser(name, regexp, script, linkName, trendName); return new DynamicParser(name, regexp, script, linkName, trendName);
} }
} else { }
else {
return null; return null;
} }
} }


public @CheckForNull AbstractWarningsParser getParser() { /**
* Returns a valid parser instance. If this parsers configuration is not valid, then <code>null</code> is returned.
*
* @return a valid parser instance or <code>null</code> if this parsers configuration is not valid.
*/
@CheckForNull
public AbstractWarningsParser getParser() {
return parser; return parser;
} }


Expand Down
Expand Up @@ -224,9 +224,8 @@ private static Iterable<GroovyParser> getDynamicParserDescriptions() {
static List<AbstractWarningsParser> getDynamicParsers(final Iterable<GroovyParser> parserDescriptions) { static List<AbstractWarningsParser> getDynamicParsers(final Iterable<GroovyParser> parserDescriptions) {
List<AbstractWarningsParser> parsers = Lists.newArrayList(); List<AbstractWarningsParser> parsers = Lists.newArrayList();
for (GroovyParser description : parserDescriptions) { for (GroovyParser description : parserDescriptions) {
AbstractWarningsParser parser = description.getParser(); if (description.isValid()) {
if (parser != null) { parsers.add(description.getParser());

This comment has been minimized.

Copy link
@jglick

jglick Feb 1, 2013

Member

Note that FindBugs will probably not grok that description.getParser() is guaranteed non-null in this case. I intentionally obtained the parser and checked for null to avoid any chance of confusion in flow analysis; if you prefer to call isValid then you may need to make getParser be marked @Nullable (i.e. “I know better so shut up”) rather than @CheckForNull.

parsers.add(parser);
} }
} }
return parsers; return parsers;
Expand Down

0 comments on commit 2987adb

Please sign in to comment.