Skip to content

Commit

Permalink
[JENKINS-46146] Ignore commented lines in the HardcodedScript Checker…
Browse files Browse the repository at this point in the history 
…, it's a global settings to be enabled/disabled globally

Change-Id: I1ef684fc8007883fc1ccf8fa3f74ba63f5bb204b
  • Loading branch information
@v1v
v1v committed Aug 11, 2017
1 parent a2a2bca commit d70dc55
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 17 deletions.
10 changes: 10 additions & 0 deletions src/main/java/org/jenkins/ci/plugins/jenkinslint/JenkinsLintGlobalConfiguration.java
View file
Expand Up @@ -45,6 +45,8 @@ public final class JenkinsLintGlobalConfiguration extends GlobalConfiguration {
private boolean windowsSlaveLaunchCheckerEnabled = true;

private int hardcodedScriptThreshold = HardcodedScriptChecker.THRESHOLD;
private boolean hardcodedScriptIgnoredComment = false;


public JenkinsLintGlobalConfiguration() {
load();
Expand Down Expand Up @@ -292,6 +294,14 @@ public void setHardcodedScriptThreshold(int hardcodedScriptThreshold) {
this.hardcodedScriptThreshold = hardcodedScriptThreshold;
}

public boolean isHardcodedScriptIgnoredComment() {
return hardcodedScriptIgnoredComment;
}

public void setHardcodedScriptIgnoredComment(boolean hardcodedScriptIgnoredComment) {
this.hardcodedScriptIgnoredComment = hardcodedScriptIgnoredComment;
}

/**
* Performs on-the-fly validation of the form field 'name'.
*
Expand Down
30 changes: 26 additions & 4 deletions src/main/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptChecker.java
View file
Expand Up @@ -17,12 +17,14 @@ public class HardcodedScriptChecker extends AbstractCheck {

public final static int THRESHOLD = 2;
private int threshold;
private boolean ignoreComment = false;

public HardcodedScriptChecker(boolean enabled, int threshold) {
public HardcodedScriptChecker(boolean enabled, int threshold, boolean ignoreComment) {
super(enabled);
this.setDescription(Messages.HardcodedScriptCheckerDesc());
this.setSeverity(Messages.HardcodedScriptCheckerSeverity());
this.setThreshold(threshold);
this.setIgnoreComment(ignoreComment);
}

public boolean executeCheck(Item item) {
Expand Down Expand Up @@ -59,7 +61,7 @@ private boolean isBuilderHarcoded (List<Builder> builders) {
if (builders != null && builders.size() > 0 ) {
for (Builder builder : builders) {
if (builder instanceof hudson.tasks.Shell || builder instanceof hudson.tasks.BatchFile) {
if (isHarcoded (((CommandInterpreter)builder).getCommand(), this.getThreshold())) {
if (isHarcoded (((CommandInterpreter)builder).getCommand(), this.getThreshold(), this.isIgnoreComment(), (builder instanceof hudson.tasks.Shell))) {
found = true;
}
}
Expand All @@ -70,12 +72,16 @@ private boolean isBuilderHarcoded (List<Builder> builders) {
return found;
}

private boolean isHarcoded (String content, int threshold) {
private boolean isHarcoded (String content, int threshold, boolean ignoreComment, boolean isUnix) {
if (content != null) {
int length = 0;
for (String line : content.split("\r\n|\r|\n")) {
if (!StringUtils.isEmptyOrBlank(line)) {
length++;
if (ignoreComment && !isACommentLine(line, isUnix)) {
length++;
} else if (!ignoreComment) {
length++;
}
}
}
return length > threshold;
Expand All @@ -84,6 +90,14 @@ private boolean isHarcoded (String content, int threshold) {
}
}

private boolean isACommentLine( String line, boolean isUnix ) {
if (isUnix) {
return StringUtils.isShellComment(line);
} else {
return StringUtils.isBatchComment(line);
}
}

public int getThreshold () {
return this.threshold;
}
Expand All @@ -92,4 +106,12 @@ public int getThreshold () {
public void setThreshold(int threshold) {
this.threshold = threshold;
}

public boolean isIgnoreComment() {
return ignoreComment;
}

public void setIgnoreComment(boolean ignoreComment) {
this.ignoreComment = ignoreComment;
}
}
4 changes: 1 addition & 3 deletions src/main/java/org/jenkins/ci/plugins/jenkinslint/check/TimerTriggerChecker.java
View file
Expand Up @@ -5,8 +5,6 @@
import hudson.triggers.TimerTrigger;
import org.jenkins.ci.plugins.jenkinslint.model.AbstractCheck;
import org.jenkins.ci.plugins.jenkinslint.utils.StringUtils;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
* @author Victor Martinez
Expand All @@ -25,7 +23,7 @@ public boolean executeCheck(Item item) {
String spec = ((AbstractProject) item).getTrigger(TimerTrigger.class).getSpec().toLowerCase();
String[] myData = spec.split("\n");
for (String line: myData) {
if (!StringUtils.isH(line) && !StringUtils.isComment(line) &&
if (!StringUtils.isH(line) && !StringUtils.isShellComment(line) &&
!StringUtils.isAt(line) && !StringUtils.isEmptyOrBlank(line)) {
found = true;
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/jenkins/ci/plugins/jenkinslint/model/AbstractAction.java
View file
Expand Up @@ -72,7 +72,8 @@ protected void reloadCheckList() {
checkList.add(new GitShallowChecker((config.isGitShallowCheckerEnabled() && config.isGlobalEnabled())));
checkList.add(new MultibranchJobTypeChecker((config.isMultibranchJobTypeCheckerEnabled() && config.isGlobalEnabled())));
checkList.add(new HardcodedScriptChecker((config.isHardcodedScriptCheckerEnabled() && config.isGlobalEnabled()),
config.getHardcodedScriptThreshold()));
config.getHardcodedScriptThreshold(),
config.isHardcodedScriptIgnoredComment()));
checkList.add(new GradleWrapperChecker((config.isGradleWrapperCheckerEnabled() && config.isGlobalEnabled())));
checkList.add(new TimeoutChecker((config.isTimeoutCheckerEnabled() && config.isGlobalEnabled())));
checkList.add(new GroovySystemExitChecker((config.isGroovySystemExitCheckerEnabled() && config.isGlobalEnabled())));
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/org/jenkins/ci/plugins/jenkinslint/utils/StringUtils.java
View file
Expand Up @@ -16,14 +16,22 @@ public static boolean isEmptyOrBlank (String line) {
return found || line.isEmpty();
}

public static boolean isComment (String line) {
public static boolean isShellComment(String line) {
boolean found = false;
Pattern p = Pattern.compile("^\\s*#\\s*.*");
Matcher m = p.matcher(line);
found = m.matches();
return found;
}

public static boolean isBatchComment(String line) {
boolean found = false;
Pattern p = Pattern.compile("^\\s*REM\\s*.*", Pattern.CASE_INSENSITIVE);
Matcher m = p.matcher(line);
found = m.matches();
return found;
}

public static boolean isH (String line) {
boolean found = false;
Pattern p = Pattern.compile("^\\s*h.*", Pattern.CASE_INSENSITIVE);
Expand Down
3 changes: 3 additions & 0 deletions .../resources/org/jenkins/ci/plugins/jenkinslint/JenkinsLintGlobalConfiguration/config.jelly
View file
Expand Up @@ -41,6 +41,9 @@
<f:entry title="${%Hardcoded Script Checker}" field="hardcodedScriptCheckerEnabled">
<f:checkbox/>
</f:entry>
<f:entry title="${%Hardcoded Script Checker - Ignore Comments}" field="hardcodedScriptIgnoredComment">
<f:checkbox/>
</f:entry>
<f:entry title="${%Hardcoded Script Checker - Threshold}" field="hardcodedScriptThreshold">
<f:textbox/>
</f:entry>
Expand Down
32 changes: 30 additions & 2 deletions src/test/java/org/jenkins/ci/plugins/jenkinslint/check/HardcodedScriptCheckerTestCase.java
View file
Expand Up @@ -16,7 +16,7 @@
* @author Victor Martinez
*/
public class HardcodedScriptCheckerTestCase extends AbstractTestCase {
private HardcodedScriptChecker checker = new HardcodedScriptChecker(true, HardcodedScriptChecker.THRESHOLD);
private HardcodedScriptChecker checker = new HardcodedScriptChecker(true, HardcodedScriptChecker.THRESHOLD, false);
private final String SINGLE_LINE_BASH = "#!/bin/bash #single line";
private final String MULTI_LINE_BASH = "#!/bin/bash\nline1\nline2\nline3\nline4\nline5\nline6";
private final String SINGLE_LINE_BATCH = "echo first";
Expand All @@ -26,6 +26,8 @@ public class HardcodedScriptCheckerTestCase extends AbstractTestCase {
private final String MULTI_SPACE_LINE2_BASH = "echo first\n \n \n \n \n \n ";
private final String MULTI_LINE_WITH_BLANK1_BASH = "echo first\n\n\n\n\n\nline1";
private final String MULTI_LINE_WITH_BLANK2_BASH = "echo first\n\n\n\n\n\nline1\nline2";
private final String MULTI_LINE_WITH_COMMENTS_SHELL = "echo first\n# \n# \n# \n# \n# \n# line1\n #";
private final String MULTI_LINE_WITH_COMMENTS_BATCH = "echo first\nREM \nREM \nREM \nREM \nREM \nREM line1\nline2";

@Test public void testDefaultJob() throws Exception {
FreeStyleProject project = j.createFreeStyleProject();
Expand Down Expand Up @@ -132,7 +134,33 @@ public class HardcodedScriptCheckerTestCase extends AbstractTestCase {
assertFalse(getMatrixProject(MULTI_LINE_WITH_BLANK1_BASH, true));
assertTrue(getMatrixProject(MULTI_LINE_WITH_BLANK2_BASH, true));
}

@Issue("JENKINS-46146")
@Test public void testJobWithCommentedLinesScript() throws Exception {
assertTrue(getJob(MULTI_LINE_WITH_COMMENTS_SHELL, true));
assertTrue(getJob(MULTI_LINE_WITH_COMMENTS_BATCH, false));
checker.setIgnoreComment(true);
assertFalse(getJob(MULTI_LINE_WITH_COMMENTS_SHELL, true));
assertFalse(getJob(MULTI_LINE_WITH_COMMENTS_BATCH, false));
checker.setIgnoreComment(false);
}
@Issue("JENKINS-46146")
@Test public void testMavenModuleWithCommentedLinesScript() throws Exception {
assertTrue(getMavenModule(MULTI_LINE_WITH_COMMENTS_SHELL, true));
assertTrue(getMavenModule(MULTI_LINE_WITH_COMMENTS_BATCH, false));
checker.setIgnoreComment(true);
assertFalse(getMavenModule(MULTI_LINE_WITH_COMMENTS_SHELL, true));
assertFalse(getMavenModule(MULTI_LINE_WITH_COMMENTS_BATCH, false));
checker.setIgnoreComment(false);
}
@Issue("JENKINS-46146")
@Test public void testMatrixProjectWithCommentedLinesScript() throws Exception {
assertTrue(getMatrixProject(MULTI_LINE_WITH_COMMENTS_SHELL, true));
assertTrue(getMatrixProject(MULTI_LINE_WITH_COMMENTS_BATCH, false));
checker.setIgnoreComment(true);
assertFalse(getMatrixProject(MULTI_LINE_WITH_COMMENTS_SHELL, true));
assertFalse(getMatrixProject(MULTI_LINE_WITH_COMMENTS_BATCH, false));
checker.setIgnoreComment(false);
}
private boolean getMatrixProject(String shell, boolean isUnix) throws Exception {
MatrixProject project = j.createMatrixProject();
if (isUnix) {
Expand Down
20 changes: 14 additions & 6 deletions src/test/java/org/jenkins/ci/plugins/jenkinslint/utils/StringUtilsTestCase.java
View file
Expand Up @@ -2,7 +2,7 @@

import org.junit.Test;
import org.jvnet.hudson.test.WithoutJenkins;
import org.jenkins.ci.plugins.jenkinslint.utils.StringUtils;

import java.lang.NullPointerException;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -39,15 +39,23 @@ public void testIsHNull() throws Exception {
StringUtils.isH(null);
}
@WithoutJenkins
@Test public void testIsComment() {
assertTrue(StringUtils.isComment("# bob"));
assertTrue(StringUtils.isComment(" # bob"));
assertFalse(StringUtils.isComment("bob # bob"));
@Test public void testIsShellComment() {
assertTrue(StringUtils.isShellComment("# bob"));
assertTrue(StringUtils.isShellComment(" # bob"));
assertFalse(StringUtils.isShellComment("bob # bob"));
}
@WithoutJenkins
@Test public void testIsBatchComment() {
assertTrue(StringUtils.isBatchComment("REM bob"));
assertTrue(StringUtils.isBatchComment("rem bob"));
assertTrue(StringUtils.isBatchComment(" REM bob"));
assertFalse(StringUtils.isBatchComment("bob REM bob"));
}
@WithoutJenkins
@Test(expected = NullPointerException.class)
public void testIsCommentNull() throws Exception {
StringUtils.isComment(null);
StringUtils.isShellComment(null);
StringUtils.isBatchComment(null);
}
@WithoutJenkins
@Test public void testIsEmptyOrBlank() {
Expand Down

0 comments on commit d70dc55

Please sign in to comment.