Avoiding Pitfalls Collaboratively Sangsoo Nam [email protected] 

Experience 

Backend Data(Json) Java Class { "name" : "Marilyn Monroe" } Jackson public class Person { public final String name; public Person(@JsonProperty("name") String name) { this.name = name; } } 

Backend Data(Json) Java Class { "name" : "Marilyn Monroe", "birth_year" : 1926 } Jackson public class Person { public final String name; public final int year; public Person(@JsonProperty("name") String name, @JsonProperty("birth_year") int year) { this.name = name; this.year = year; } } 

Crash…!? 

Good! Crash! 

Backend Data(Json) Java Class { "name" : "Marilyn Monroe", "birth_year" : 1926 } public class Person { public final String name; public final int year; public Person(@JsonProperty("name") String name, @JsonProperty("birth_year") int year) { this.name = name; } } Java Class public class Person { public final String name; public Person(@JsonProperty("name") String name) { this.name = name; } } Jackson 

Backend Data(Json) Java Class { "name" : "Marilyn Monroe", "birth_year" : 1926 } public class Person { public final String name; public final int year; public Person(@JsonProperty("name") String name, @JsonProperty("birth_year") int year) { this.name = name; } } Java Class public class Person { public final String name; public Person(@JsonProperty("name") String name) { this.name = name; } } Jackson UnrecognizedPropertyException 

Backend Data(Json) Java Class { "name" : "Marilyn Monroe", "birth_year" : 1926 } public class Person { public final String name; public final int year; public Person(@JsonProperty("name") String name, @JsonProperty("birth_year") int year) { this.name = name; } } Java Class @JsonIgnoreProperties(ignoreUnknown = true) public class Person { public final String name; public Person(@JsonProperty("name") String name) { this.name = name; } } Jackson 

How to share your knowledge? 

Experience > Knowledge 

What if others will see …? 

No content

Brief Description Location 

Brief Description Location Explanation 

Android Custom Lint (Static Code Analysis Tool) ./gradlew lint 

Agenda • Key Concepts (Issue, Detector and IssueRegistry) • Practical Examples • Tips • Setup & Test 

Issue, Detector and IssueRegistry 

Issue A definition of problem you want to show public static Issue ISSUE = Issue.create( "JacksonIgnoreProperties", // ID "Should use @JsonIgnoreProperties(ignoreUnknown = true)", // Brief Description "By default, @JsonIgnoreProperties(ignoreUnknown = false)...", // Explanation Category.CORRECTNESS, // Category 6, // Priority Severity.ERROR, // Severity new Implementation( JacksonIgnorePropertiesDetector.class, // Detector Scope.JAVA_FILE_SCOPE // Scope ) ); 

Responsible to find issues and report Detector ClassScanner JavaScanner XmlScanner Detector … 

Responsible to find issues and report Detector ClassScanner JavaScanner XmlScanner Java File Scope Class File Scope Manifest File or Resource File Scope 

IssueRegistry Provides a list of issues public class CustomIssueRegistry extends IssueRegistry { @Override public List getIssues() { return Arrays.asList( JacksonIgnorePropertiesJavaDetector.ISSUE ); } } 

Practical Examples 

@JsonIgnoreProperties 

We want to avoid public class Person { public final String name; public Person(@JsonProperty("name") String name) { this.name = name; } } We don’t care public class Person { public final String name; public Person(String name) { this.name = name; } } 

We expect @JsonIgnoreProperties(ignoreUnknown = true) public class Person { public final String name; public Person(@JsonProperty("name") String name) { this.name = name; } } @JsonProperty ignoreUnknown = true Constructor 

Issue with Java Scope public static Issue ISSUE = Issue.create( "JacksonIgnoreProperties", // ID "Should use @JsonIgnoreProperties(ignoreUnknown = true)", // Brief Description "By default, @JsonIgnoreProperties(ignoreUnknown = false)...", // Explanation Category.CORRECTNESS, // Category 6, // Priority Severity.ERROR, // Severity new Implementation( JacksonIgnorePropertiesDetector.class, // Detector Scope.Java_FILE_SCOPE // Scope ) ); 

Detector with Java Scanner public class JacksonIgnorePropertiesDetector extends Detector implements Detector.JavaScanner { public static Issue ISSUE = {...} @Override public AstVisitor createJavaVisitor(JavaContext context) { return new JavaVisitor(context); } private static class JavaVisitor extends ForwardingAstVisitor { public JavaVisitor(JavaContext context) {...} @Override public boolean visitClassDeclaration(ClassDeclaration node) {...} @Override public boolean visitConstructorDeclaration(ConstructorDeclaration node) {...} } } 

visitClassDeclaration @Override public boolean visitClassDeclaration(ClassDeclaration node) { for (Annotation annotation : node.astModifiers().astAnnotations()) { if (annotation.astAnnotationTypeReference().getTypeName() .equals("JsonIgnoreProperties")) { for (Node valueNode : annotation.getValues("ignoreUnknown")) { if (valueNode instanceof BooleanLiteral && Objects.equals(true, ((BooleanLiteral) valueNode).astValue())) { mHasJsonIgnorePropertiesAnnotationWithTrueValue = true; return false; } } } } return false; } 

No content

visitConstructorDeclaration @Override public boolean visitConstructorDeclaration(ConstructorDeclaration node) { boolean hasJsonPropertyAnnotation = false; LoopVariables: for (VariableDefinition variableDefinition : node.astParameters()) { for (Annotation annotation : variableDefinition.astModifiers().astAnnotations()) { if (annotation.astAnnotationTypeReference().getTypeName().equals("JsonProperty")) { hasJsonPropertyAnnotation = true; break LoopVariables; } } } if (hasJsonPropertyAnnotation && !mHasJsonIgnorePropertiesAnnotationWithTrueValue) { mContext.report(ISSUE, mContext.getLocation(node.getParent()), "Should use @JsonIgnoreProperties(ignoreUnknown = true)"); // Message } return true; } 

@NonNull and @Nullable 

We want to avoid public class Person { public final String name; public final int age; public Person(@JsonProperty("name") String name, @JsonProperty("age") int age) { this.name = name; this.age = age; } } We don’t care public class Person { public final String name; public final int age; public Person(String name, int age) { this.name = name; this.age = age; } } 

We expect public class Person { public final String name; public final int age; public Person(@NonNull @JsonProperty("name") String name, @JsonProperty("age") int age) { this.name = name; this.age = age; } } Use @NonNull or @Nullable Don’t care a primitive type 

Issue with Java Scope public static Issue ISSUE = Issue.create( "JacksonConstructorWithNonNullOrNullable", "Should use @NotNull or @Nullable annotation", "Without @NonNull or @Nullable annotations, it is not obvious to say a value is " + "not null or nullable. It could make any wrong assumption. To avoid that, " + "annotations should be used", Category.CORRECTNESS, 6, Severity.ERROR, new Implementation( JacksonConstructorNonNullJavaDetector.class, Scope.JAVA_FILE_SCOPE ) ); 

Detector with Java Scanner private static class JavaVisitor extends ForwardingAstVisitor { @Override public boolean visitConstructorDeclaration(ConstructorDeclaration node) { for (VariableDefinition variableDefinition : node.astParameters()) { if (variableDefinition.astTypeReference().isPrimitive()) { // Skip primitive value continue; } Set typeNames = new HashSet<>(); for (Annotation annotation : variableDefinition.astModifiers().astAnnotations()) { typeNames.add(annotation.astAnnotationTypeReference().getTypeName()); } if (typeNames.contains("JsonProperty")) { if (typeNames.contains("NonNull") || typeNames.contains("Nullable")) { continue; } mContext.report(ISSUE, mContext.getLocation(variableDefinition), "Should use @NotNull or @Nullable annotation"); } } return false; } } 

Register and Unregister 

FollowService.java public class FollowService { private Set mListeners = new HashSet<>(); public void registerListener(Listener listener) { mListeners.add(listener); } public void unregisterListener(Listener listener) { mListeners.remove(listener); } ... } 

We want to avoid public class FollowFragment extends Fragment{ private FollowService mFollowService; ... @Override public void onStart() { super.onStart(); mFollowService.registerListener(mListener); } @Override public void onStop() { super.onStop(); } } 

We expect public class FollowFragment extends Fragment{ private FollowService mFollowService; ... @Override public void onStart() { super.onStart(); mFollowService.registerListener(mListener); } @Override public void onStop() { super.onStop(); mFollowService.unregisterListener(mListener); } } register unregister 

Issue with Class Scope public static Issue ISSUE = Issue.create( "RegisterUnregister", "Should call registerListener() and unregisterListener() in the same class.", "To avoid memory leak, the class which call registerListener() should also call " + "unregisterListener(). vice versa.", Category.CORRECTNESS, 6, Severity.ERROR, new Implementation( RegisterUnregisterClassDetector.class, Scope.CLASS_FILE_SCOPE ) ); 

We expect public class FollowFragment extends Fragment{ private FollowService mFollowService; ... @Override public void onStart() { super.onStart(); mFollowService.registerListener(mListener); } @Override public void onStop() { super.onStop(); mFollowService.unregisterListener(mListener); } } Call Name Call Owner 

Detector with Class Scanner public class RegisterUnregisterClassDetector extends Detector implements Detector.ClassScanner { public static Issue ISSUE = {...} private boolean mIsRegisterCalled; private boolean mIsUnregisterCalled; @Override public List getApplicableCallNames() { return Lists.newArrayList("registerListener", "unregisterListener"); } @Override public List getApplicableCallOwners() { return Lists.newArrayList("registerunregister/FollowService"); } @Override public void checkCall(ClassContext context, ClassNode cn, MethodNode m, MethodInsnNode call) { if (call.owner.equals("registerunregister/FollowService")) { if (call.name.equals("registerListener")) { mIsRegisterCalled = true; } else if (call.name.equals("unregisterListener")) { mIsUnregisterCalled = true; } } } 

Detector with Class Scanner @Override public void afterCheckFile(Context context) { super.afterCheckFile(context); ClassContext classContext = (ClassContext) context; // If one is false. if (mIsRegisterCalled ^ mIsUnregisterCalled) { if (!mIsRegisterCalled) { context.report(ISSUE, classContext.getLocation(classContext.getClassNode()), "registerListener() should be called in the same class."); } else { context.report(ISSUE, classContext.getLocation(classContext.getClassNode()), "unregisterListener() should be called in the same class."); } } } } 

String Resource Sorting by Name 

We want to avoid Welcome Hello! %d song %d songs 

We expect Hello! %d song %d songs Welcome Sorted by Name: Alphabetically 

Issue with Resource Scope public static Issue ISSUE = Issue.create( "StringsSortedByName", "Strings should be ordered by name.", "To keep consistency, strings should be ordered by name.", Category.CORRECTNESS, 6, Severity.ERROR, new Implementation( StringsSortedByNameResourceDetector.class, Scope.RESOURCE_FILE_SCOPE ) ); 

Detector with Xml Scanner public class StringsSortedByNameResourceDetector extends Detector implements Detector.XmlScanner { public static Issue ISSUE = {...} @Override public boolean appliesTo(ResourceFolderType folderType) { return folderType == ResourceFolderType.VALUES; } @Override public Collection getApplicableElements() { return Arrays.asList(SdkConstants.TAG_PLURALS, SdkConstants.TAG_STRING, SdkConstants.TAG_STRING_ARRAY); } @Override public void visitElement(XmlContext context, Element element) {...} } 

visitElement @Override public void visitElement(XmlContext context, Element element) { Node node = element.getNextSibling(); while (node != null && node.getNodeType() != Node.ELEMENT_NODE) { node = node.getNextSibling(); } if (node == null) { // No next element to compare return; } Element nextElement = (Element) node; String name = element.getAttribute("name"); String nextName = nextElement.getAttribute("name"); if (name.compareTo(nextName) > 0) { context.report(ISSUE, context.getLocation(element), "Resource element name (" + nextName + ") must be sorted after (" + name + ")."); } } 

Tips 

Source Code Best documents you can use @Beta public abstract class Detector { /** Specialized interface for detectors that scan Java source file parse trees */ public interface JavaScanner { /** * Create a parse tree visitor to process the parse tree. All * {@link JavaScanner} detectors must provide a visitor, unless they * either return true from {@link #appliesToResourceRefs()} or return * non null from {@link #getApplicableMethodNames()} .... * @param context the {@link Context} for the file being analyzed * @return a visitor, or null. */ @Nullable AstVisitor createJavaVisitor(@NonNull JavaContext context); 

Source Code Some methods should be overridden to work properly public interface ClassScanner { void checkClass(@NonNull ClassContext context, @NonNull ClassNode classNode); int[] getApplicableAsmNodeTypes(); void checkInstruction(@NonNull ClassContext context, @NonNull ClassNode classNode, @NonNull MethodNode method, @NonNull AbstractInsnNode instruction); List getApplicableCallNames(); List getApplicableCallOwners(); void checkCall(@NonNull ClassContext context, @NonNull ClassNode classNode, @NonNull MethodNode method, @NonNull MethodInsnNode call); } 

Use Debugger The best way to check the value and to know how to access 

Severity.ERROR People tend to ignore warnings public static Issue ISSUE = Issue.create( "JacksonIgnoreProperties", // ID "Should use @JsonIgnoreProperties(ignoreUnknown = true)", // Brief Description "By default, @JsonIgnoreProperties(ignoreUnknown = false)..." + // Explanation Category.CORRECTNESS, // Category 6, // Priority Severity.ERROR, // Severity new Implementation( JacksonIgnorePropertiesDetector.class, // Detector Scope.Java_FILE_SCOPE // Scope ) ); 

Setup & Tests 

Github Repository https:/ /github.com/SangsooNam/CustomAndroidLint 

lint.jar jar { archiveName 'lint.jar' manifest { attributes("Lint-Registry": "io.github.sangsoonam.lint.CustomIssueRegistry") } } Build a jar file having lint registry attribute 

CopyLintJar configurations { customLint } task copyLintJar(type: Copy) { from (configurations.customLint) into "$buildDir/lint" } project.afterEvaluate { it.tasks.compileLint.dependsOn copyLintJar } dependencies { customLint project(path:':customlint', configuration: 'archives') } Copied lint.jar is used while ./gradlew lint 

Sample Module For Testing Where can you get class files for testing? project.afterEvaluate { tasks.compileTestJava.dependsOn ':customlint:sample:assemble' } 

Conclusion 

Android Custom Lint Static Code Analysis Tool Detector (JavaScanner) Issue (Java File Scope) Issue Registry Detector (ClassScanner) Issue (Class File Scope) Detector (XmlScanner) Issue (Resource File Scope) 

No content

Brief Description Location 

Brief Description Location Explanation 

Questions Sangsoo Nam [email protected] Join the band! www.spotify.com/jobs 

Let’s avoid pitfalls together