Skip to content
Permalink
Browse files
Add tabfiles to jcheck configuration
Reviewed-by: erikj, ehelin
  • Loading branch information
magicus authored and rwestberg committed Nov 16, 2020
1 parent f4dc83b commit d468590c8d37013075d19b4d3fe76ee1eb419f46
Showing 3 changed files with 86 additions and 23 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -26,11 +26,8 @@
import org.openjdk.skara.vcs.Commit;
import org.openjdk.skara.vcs.openjdk.CommitMessage;

import java.io.IOException;
import java.nio.file.Path;
import java.util.*;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import java.util.logging.Logger;

public class WhitespaceCheck extends CommitCheck {
@@ -41,6 +38,7 @@ Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration
var metadata = CommitIssue.metadata(commit, message, conf, this);
var issues = new ArrayList<Issue>();
var pattern = Pattern.compile(conf.checks().whitespace().files());
var tabPattern = Pattern.compile(conf.checks().whitespace().ignoreTabs());

for (var diff : commit.parentDiffs()) {
for (var patch : diff.patches()) {
@@ -56,13 +54,15 @@ Iterator<Issue> check(Commit commit, CommitMessage message, JCheckConfiguration
var row = hunk.target().range().start() + i;
var tabIndex = line.indexOf('\t');
var crIndex = line.indexOf('\r');
if (tabIndex >= 0 || crIndex >= 0 || line.endsWith(" ")) {
var ignoreTab = tabPattern.matcher(path.toString()).matches();
if ((tabIndex >= 0 && !ignoreTab) || crIndex >= 0
|| line.endsWith(" ") || line.endsWith("\t")) {
var errors = new ArrayList<WhitespaceIssue.Error>();
var trailing = true;
for (var index = line.length() - 1; index >= 0; index--) {
if (line.charAt(index) == ' ' && trailing) {
if ((line.charAt(index) == ' ' || line.charAt(index) == '\t') && trailing) {
errors.add(WhitespaceIssue.trailing(index));
} else if (line.charAt(index) == '\t') {
} else if (line.charAt(index) == '\t' && !ignoreTab) {
errors.add(WhitespaceIssue.tab(index));
} else if (line.charAt(index) == '\r') {
errors.add(WhitespaceIssue.cr(index));
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -24,24 +24,26 @@

import org.openjdk.skara.ini.Section;

import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

public class WhitespaceConfiguration {
static final WhitespaceConfiguration DEFAULT =
new WhitespaceConfiguration(".*\\.cpp|.*\\.hpp|.*\\.c|.*\\.h|.*\\.java");
new WhitespaceConfiguration(".*\\.cpp|.*\\.hpp|.*\\.c|.*\\.h|.*\\.java", "");

private final String files;
private final String ignoreTabs;

WhitespaceConfiguration(String files) {
WhitespaceConfiguration(String files, String ignoreTabs) {
this.files = files;
this.ignoreTabs = ignoreTabs;
}

public String files() {
return files;
}

public String ignoreTabs() {
return ignoreTabs;
}

static String name() {
return "whitespace";
}
@@ -52,6 +54,7 @@ static WhitespaceConfiguration parse(Section s) {
}

var files = s.get("files", DEFAULT.files());
return new WhitespaceConfiguration(files);
var ignoreTabs = s.get("ignore-tabs", DEFAULT.ignoreTabs());
return new WhitespaceConfiguration(files, ignoreTabs);
}
}
@@ -22,7 +22,6 @@
*/
package org.openjdk.skara.jcheck;

import org.openjdk.skara.census.Census;
import org.openjdk.skara.vcs.*;
import org.openjdk.skara.vcs.openjdk.CommitMessage;
import org.openjdk.skara.vcs.openjdk.CommitMessageParsers;
@@ -35,7 +34,6 @@
import java.util.List;
import java.util.ArrayList;
import java.time.ZonedDateTime;
import java.io.IOException;

class WhitespaceCheckTests {
private static List<Diff> parentDiffs(String filename, String line) {
@@ -56,9 +54,10 @@ private static List<Diff> parentDiffs(String filename, String line) {
"[checks \"whitespace\"]"
);

private static JCheckConfiguration configuration(String files) {
private static JCheckConfiguration configuration(String files, String ignoreTabs) {
var lines = new ArrayList<>(CONFIGURATION);
lines.add("files = " + files);
lines.add("ignore-tabs = " + ignoreTabs);
return JCheckConfiguration.parse(lines);
}

@@ -87,7 +86,7 @@ private List<Issue> toList(Iterator<Issue> i) {
@Test
void noBadWhitespaceShouldPass() {
var commit = commit(parentDiffs("README.md", "An additional line"));
var conf = configuration("README\\.md");
var conf = configuration("README\\.md", "");
var check = new WhitespaceCheck();
var issues = toList(check.check(commit, message(commit), conf, null));

@@ -99,7 +98,7 @@ void trailingWhitespaceShouldFail() {
var filename = "README.md";
var line = "An additional line ";
var commit = commit(parentDiffs(filename, line));
var conf = configuration("README\\.md");
var conf = configuration("README\\.md", "");
var message = message(commit);
var check = new WhitespaceCheck();
var issues = toList(check.check(commit, message, conf, null));
@@ -119,11 +118,59 @@ void trailingWhitespaceShouldFail() {
}

@Test
void tabShouldFail() {
void trailingTabShouldFailWithoutIgnoreTabs() {
var filename = "README.md";
var line = "An additional line\t";
var commit = commit(parentDiffs(filename, line));
var conf = configuration("README\\.md", "");
var message = message(commit);
var check = new WhitespaceCheck();
var issues = toList(check.check(commit, message, conf, null));

assertEquals(1, issues.size());
assertTrue(issues.get(0) instanceof WhitespaceIssue);
var issue = (WhitespaceIssue) issues.get(0);
assertEquals(Path.of(filename), issue.path());
assertEquals(1, issue.row());
assertEquals(line, issue.line());
assertEquals(List.of(new WhitespaceIssue.Error(line.length() - 1, WhitespaceIssue.Whitespace.TRAILING)),
issue.errors());
assertEquals(commit, issue.commit());
assertEquals(check, issue.check());
assertEquals(message, issue.message());
assertEquals(Severity.ERROR, issue.severity());
}

@Test
void trailingTabShouldFailWithIgnoreTabs() {
var filename = "README.md";
var line = "An additional line\t";
var commit = commit(parentDiffs(filename, line));
var conf = configuration("README\\.md", "\"README\\\\.md");
var message = message(commit);
var check = new WhitespaceCheck();
var issues = toList(check.check(commit, message, conf, null));

assertEquals(1, issues.size());
assertTrue(issues.get(0) instanceof WhitespaceIssue);
var issue = (WhitespaceIssue) issues.get(0);
assertEquals(Path.of(filename), issue.path());
assertEquals(1, issue.row());
assertEquals(line, issue.line());
assertEquals(List.of(new WhitespaceIssue.Error(line.length() - 1, WhitespaceIssue.Whitespace.TRAILING)),
issue.errors());
assertEquals(commit, issue.commit());
assertEquals(check, issue.check());
assertEquals(message, issue.message());
assertEquals(Severity.ERROR, issue.severity());
}

@Test
void tabShouldFailWithoutIgnoreTabs() {
var filename = "README.md";
var line = "\tAn additional line";
var commit = commit(parentDiffs(filename, line));
var conf = configuration("README\\.md");
var conf = configuration("README\\.md", "");
var message = message(commit);
var check = new WhitespaceCheck();
var issues = toList(check.check(commit, message, conf, null));
@@ -142,12 +189,25 @@ void tabShouldFail() {
assertEquals(Severity.ERROR, issue.severity());
}

@Test
void tabShouldSucceedWithIgnoreTabs() {
var filename = "README.md";
var line = "\tAn additional line";
var commit = commit(parentDiffs(filename, line));
var conf = configuration("README\\.md", "README\\.md");
var message = message(commit);
var check = new WhitespaceCheck();
var issues = toList(check.check(commit, message, conf, null));

assertEquals(0, issues.size());
}

@Test
void crShouldFail() {
var filename = "README.md";
var line = "An additional line\r\n";
var commit = commit(parentDiffs(filename, line));
var conf = configuration("README\\.md");
var conf = configuration("README\\.md", "");
var message = message(commit);
var check = new WhitespaceCheck();
var issues = toList(check.check(commit, message, conf, null));

1 comment on commit d468590

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on d468590 Nov 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.