Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: fix more checkstyle violations #2127

Merged
merged 2 commits into from
Jun 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/test/java/spoon/test/api/NoClasspathTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void test() throws Exception {
}

{
CtMethod<?> method = clazz.getMethod("m3",new CtTypeReference[0]);
CtMethod<?> method = clazz.getMethod("m3", new CtTypeReference[0]);
assertNotNull(method);
List<CtInvocation<?>> invocations = method.getElements(new TypeFilter<CtInvocation<?>>(CtInvocation.class));
assertEquals(1, invocations.size());
Expand All @@ -108,7 +108,6 @@ public void test() throws Exception {
assertEquals("field", fa.getVariable().getSimpleName());
assertEquals("int x = first().field", statement.toString());
}

}

@Test
Expand Down Expand Up @@ -170,13 +169,13 @@ public void testInheritanceInNoClassPathWithClasses() throws IOException {
spoon = new Launcher();
spoon.getEnvironment().setNoClasspath(true);
spoon.getEnvironment().setSourceClasspath(new String[] { targetBinPath });
spoon.addInputResource(sourceInputDirPath+"/AnotherClass.java");
spoon.addInputResource(sourceInputDirPath + "/AnotherClass.java");
spoon.buildModel();

CtType anotherclass = spoon.getFactory().Type().get("org.acme.AnotherClass");
assertEquals(1, anotherclass.getFields().size());

CtField field = (CtField)anotherclass.getFields().get(0);
CtField field = (CtField) anotherclass.getFields().get(0);

CtTypeReference myClassReference = spoon.getFactory().Type().createReference("fr.acme.MyClass");
assertEquals(myClassReference, field.getType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,19 @@ public boolean matches(CtType element) {
};
})) {
for (Object o : t.getFields()) {
CtField f=(CtField)o;
if (f.getSimpleName().equals("factory")) { continue; }
if (f.hasModifier(ModifierKind.FINAL) || f.hasModifier(ModifierKind.TRANSIENT) ) { continue; }
CtField f = (CtField) o;
if (f.getSimpleName().equals("factory")) {
continue;
}
if (f.hasModifier(ModifierKind.FINAL) || f.hasModifier(ModifierKind.TRANSIENT)) {
continue;
}

fail("architectural constraint: a factory must be stateless");
}
}

}


@Test
public void testFactorySubFactory() throws Exception {
// contract:: all subfactory methods must also be in the main factory
Expand All @@ -79,28 +81,38 @@ public void process() {
CtInterface itf = getFactory().Interface().create("MegaFactoryItf");
CtClass impl = getFactory().Class().create("MegaFactory");
for (CtType<?> t : factoryPackage.getTypes()) {
if (t.getSimpleName().startsWith("Mega")) continue; //
if (t.getSimpleName().startsWith("Mega")) {
continue;
}

for (CtMethod<?> m : t.getMethods()) {
// we check only public methods
if (m.hasModifier(ModifierKind.PUBLIC) == false) continue;
if (m.hasModifier(ModifierKind.PUBLIC) == false) {
continue;
}
// we only consider factory methods
if (!m.getSimpleName().startsWith("create")) continue;
if (!m.getSimpleName().startsWith("create")) {
continue;
}

// too generic, what should we create??
if (m.getSimpleName().equals("create")) {
String simpleNameType = m.getType().getSimpleName().replace("Ct", "");
CtMethod method = m.clone();

method.setSimpleName("create"+simpleNameType);
assertTrue(method.getSignature() + " (from "+t.getQualifiedName()+") is not present in the main factory", factoryImpl.hasMethod(method));
method.setSimpleName("create" + simpleNameType);
assertTrue(method.getSignature() + " (from " + t.getQualifiedName() + ") is not present in the main factory", factoryImpl.hasMethod(method));
continue;
}

// too generic, is it a fieldref? an execref? etc
if (m.getSimpleName().equals("createReference"))
if (m.getSimpleName().equals("createReference")) {
continue;
}

if (m.getModifiers().contains(ModifierKind.ABSTRACT)) continue;
if (m.getModifiers().contains(ModifierKind.ABSTRACT)) {
continue;
}

sanityCheck.val++;

Expand Down Expand Up @@ -160,10 +172,9 @@ public void testSrcMainJava() throws Exception {
}
}
if (notDocumented.size() > 0) {
fail(notDocumented.size()+" public methods should be documented with proper API documentation: \n"+StringUtils.join(notDocumented, "\n"));
fail(notDocumented.size() + " public methods should be documented with proper API documentation: \n" + StringUtils.join(notDocumented, "\n"));
}


// contract: Spoon's code never uses TreeSet constructor, because they implicitly depend on Comparable (no static check, only dynamic checks)
List<CtConstructorCall> treeSetWithoutComparators = spoon.getFactory().Package().getRootPackage().filterChildren(new AbstractFilter<CtConstructorCall>() {
@Override
Expand Down Expand Up @@ -203,7 +214,6 @@ public void metamodelPackageRule() throws Exception {
}
}


@Test
public void testGoodTestClassNames() throws Exception {
// contract: to be run by Maven surefire, all test classes must be called Test* or *Test
Expand All @@ -220,13 +230,13 @@ public boolean matches(CtMethod element) {
return super.matches(element) && element.getAnnotation(Test.class) != null;
}
})) {
assertTrue("naming contract violated for "+meth.getParent(CtClass.class).getSimpleName(), meth.getParent(CtClass.class).getSimpleName().startsWith("Test") || meth.getParent(CtClass.class).getSimpleName().endsWith("Test"));
assertTrue("naming contract violated for " + meth.getParent(CtClass.class).getSimpleName(), meth.getParent(CtClass.class).getSimpleName().startsWith("Test") || meth.getParent(CtClass.class).getSimpleName().endsWith("Test"));
}

// contract: the Spoon test suite does not depend on Junit 3 classes and methods
// otherwise, intellij automatically selects the junit3 runner, finds nothing
// and crashes with a dirty exception
assertEquals(0, spoon.getModel().getElements(new TypeFilter<CtTypeReference>(CtTypeReference.class){
assertEquals(0, spoon.getModel().getElements(new TypeFilter<CtTypeReference>(CtTypeReference.class) {
@Override
public boolean matches(CtTypeReference element) {
CtMethod parent = element.getParent(CtMethod.class);
Expand Down Expand Up @@ -280,8 +290,8 @@ public void testStaticClasses() throws Exception {
for (CtClass<?> klass : spoon.getModel().getElements(new TypeFilter<CtClass>(CtClass.class) {
@Override
public boolean matches(CtClass element) {
return element.getSuperclass() == null && super.matches(element) && element.getMethods().size()>0
&& element.getElements(new TypeFilter<>(CtMethod.class)).stream().allMatch( x -> x.hasModifier(ModifierKind.STATIC));
return element.getSuperclass() == null && super.matches(element) && element.getMethods().size() > 0
&& element.getElements(new TypeFilter<>(CtMethod.class)).stream().allMatch(x -> x.hasModifier(ModifierKind.STATIC));
}
})) {
assertTrue(klass.getElements(new TypeFilter<>(CtConstructor.class)).stream().allMatch(x -> x.hasModifier(ModifierKind.PRIVATE)));
Expand Down Expand Up @@ -398,34 +408,33 @@ public void process(CtPackage element) {
assertSetEquals("you have created a new package or removed an existing one, please declare it explicitly in SpoonArchitectureEnforcerTest#testSpecPackage", officialPackages, currentPackages);
}

private static void assertSetEquals(String msg, Set<?> set1, Set<?> set2){
if(set1 == null || set2 ==null){
private static void assertSetEquals(String msg, Set<?> set1, Set<?> set2) {
if (set1 == null || set2 == null) {
throw new IllegalArgumentException();
}

if(set1.size() != set2.size()){
throw new AssertionError(msg+"\n\nDetails: "+computeDifference(set1, set2));
if (set1.size() != set2.size()) {
throw new AssertionError(msg + "\n\nDetails: " + computeDifference(set1, set2));
}

if (!set1.containsAll(set2)) {
throw new AssertionError(msg+"\n\nDetails: "+computeDifference(set1, set2));
throw new AssertionError(msg + "\n\nDetails: " + computeDifference(set1, set2));
}

}

private static String computeDifference(Set<?> set1, Set<?> set2) {
Set<String> results = new HashSet<>();

for (Object o : set1) {
if (!set2.contains(o)) {
results.add("Missing package "+o+" in computed set");
results.add("Missing package " + o + " in computed set");
} else {
set2.remove(o);
}
}

for (Object o : set2) {
results.add("Package "+o+" presents in computed but not expected set.");
results.add("Package " + o + " presents in computed but not expected set.");
}
return StringUtils.join(results, "\n");
}
Expand Down