Take care of revision: code analysis automation techniques
A code review is not some kind of formal measure, but an invaluable opportunity for novice programmers to quickly delve into the specifics of the project. But this stage is connected with the risks of hemorrhage in the eyes due to the amount of materials that the reviewers have to view and think over.
In this article, we will tell you how to remove from reviewers such trivial, but important things as checking for compliance with the accepted style of writing code, compliance with agreements that are specific only for a specific project. As an example, we will take the code of Android applications, but the technique can be used for any Java code.
As the project grows, many agreements on the implementation of individual modules, instructions on the use of libraries, their own frameworks are accumulated. Let us give examples of such agreements:
Open resource after use must be closed. Otherwise, we get a leak of resources and related problems. Many static analysis tools can do this checking for known resources. Frequent resources are sockets, database connections, memory areas. But you can add a check for your specific resources.
')
For example, in all methods where there is access to the database, before accessing it, you must call a specific audit function with the necessary parameters.
We know that both hashCode () and equals () in successor classes should always be overridden. You may have similar agreements in your project. Another example from this category is checking that the variables you expect are used in the overridden method. This can be used to verify that the method is being used as intended.
In fairness, we add that verification is another level of control of agreements. Ideally, the application architecture should be such that it is difficult to break, but this is a topic for another article.
We will check that the Android application uses one logger, ru.project.Logger, and also does not use the android.util.Log class. This is necessary so that in the release application it was possible in one place to turn off the output to the log of security-sensitive information.
The bad news is that popular FindBugs has not developed since 2015. Good - there is a SpotBugs : "
How does he work? The analyzer is sent to the input directory or selected file with bytecode. Bytecode analysis uses the Apache BCEL library. Direct checks are implemented in the successors of the Detector class. A detector is a visitor that “enters” classes, methods, and method code. There are many heirs to this interface in the FindBugs code.
1. We extort the spotbugs code.
git clone
2. We collect the project
./gradlew distZip
3. Unpacking the distribution kit in the /spotbugs-4.0.0-SNAPSHOT directory
To develop a plugin, we use the intellij idea community edition.
4. Create a new java (gradle) application project.
5. We adjusted dependences. Copy the spotbugs.jar file to the project / libs directory.
Findbugs.xml file - plugin description
Files Messages.xml - found bug message template
Recall that the detector works with bytecode. To understand the detector's work, let us present a part of the bytecode of the onCreate () Activity method, where Lod.d is called (“aaa”, “bbb”);
Line 9 will search for our detector. This is a static method call in the android.util.Log class.
We assemble the plugin:
Copy the file of the plugin detector-1.0-SNAPSHOT.jar in /spotbugs-4.0.0-SNAPSHOT/plugin. Run ui interface spotbugs /spotbugs-4.0.0-SNAPSHOT/bin/spotbugs. Select the bytecode directory of the test android project for verification. / TestAndroidApp / app / build / intermediates / classes / debug
As you can see, there were all calls to the android.util.Log class. Now our custom detector can be used in the CI system. He will inform the developer about the bugs found.
Now we will free the reviewer from checking the code for compliance with the code-style. For this we will use the Checkstyle plugin . To him, by the way, is attached a list of all the checks that have already been implemented, is here .
Plugin is configured using the checkstyle.xml configuration file. It indicates the necessary checks and their parameters. We give examples of the file for the following checks:
A set of ready-made rules and the ability to configure them allow you to check almost any requirements. But if you need to create your own special rule, this can also be done according to the instructions .
Note that Checkstyle works with the source code of the project, the analysis of which was done using ANTLR . To implement the verification, you must implement the visitor, which is traversed through the syntax tree of the source code. In this article we will not consider the creation of a custom code-style check.
Add lines to build.gradle
Run the check ./gradlew checkstyle.
The report will be in the /build/reports/checkstyle/checkstyle.html directory. Here is an example report.
But the code that was checked
So, we have freed revisers from the routine work - checking the code-style and checking the project agreements. Now developers will have more time for creativity, and the code will become better. We hope the material was useful, we will be happy to answer your questions in the comments.
In this article, we will tell you how to remove from reviewers such trivial, but important things as checking for compliance with the accepted style of writing code, compliance with agreements that are specific only for a specific project. As an example, we will take the code of Android applications, but the technique can be used for any Java code.
As the project grows, many agreements on the implementation of individual modules, instructions on the use of libraries, their own frameworks are accumulated. Let us give examples of such agreements:
- Resource management.
Open resource after use must be closed. Otherwise, we get a leak of resources and related problems. Many static analysis tools can do this checking for known resources. Frequent resources are sockets, database connections, memory areas. But you can add a check for your specific resources.
')
- The order of the function call in the method.
For example, in all methods where there is access to the database, before accessing it, you must call a specific audit function with the necessary parameters.
- Agreement on the implementation of the heirs of abstract classes.
We know that both hashCode () and equals () in successor classes should always be overridden. You may have similar agreements in your project. Another example from this category is checking that the variables you expect are used in the overridden method. This can be used to verify that the method is being used as intended.
In fairness, we add that verification is another level of control of agreements. Ideally, the application architecture should be such that it is difficult to break, but this is a topic for another article.
We will check that the Android application uses one logger, ru.project.Logger, and also does not use the android.util.Log class. This is necessary so that in the release application it was possible in one place to turn off the output to the log of security-sensitive information.
What will we check
The bad news is that popular FindBugs has not developed since 2015. Good - there is a SpotBugs : "
How does he work? The analyzer is sent to the input directory or selected file with bytecode. Bytecode analysis uses the Apache BCEL library. Direct checks are implemented in the successors of the Detector class. A detector is a visitor that “enters” classes, methods, and method code. There are many heirs to this interface in the FindBugs code.
Setting up the environment for developing the FindBugs plugin
1. We extort the spotbugs code.
git clone
2. We collect the project
./gradlew distZip
3. Unpacking the distribution kit in the /spotbugs-4.0.0-SNAPSHOT directory
To develop a plugin, we use the intellij idea community edition.
4. Create a new java (gradle) application project.
5. We adjusted dependences. Copy the spotbugs.jar file to the project / libs directory.
build.gradle dependencies { compile fileTree(dir: 'libs', include: ['*.jar']) compile 'org.apache.bcel:bcel:6.1' } What is the plugin
Findbugs.xml file - plugin description
<FindbugsPlugin xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://findbugs.googlecode.com/svn-history/r13379/trunk/findbugs/etc/findbugsplugin.xsd" pluginid="ru.sberbank.android.log.plugin" provider="Find Android.Util.Log class" defaultenabled="true" website="https://find-sec-bugs.imtqy.com"> <Detector class="ru.sbt.detector.AndroidLogCallDetector"/> <BugPattern type="ANDROID_LOG_CALL_DETECTED" abbrev="ANDR_LOG" category="CORRECTNESS"/> </FindbugsPlugin> Files Messages.xml - found bug message template
<MessageCollection> <Detector class="ru.sbt.detector.AndroidLogCallDetector"> <Details>ANDROID_LOG_CALL_DETECTED alala.</Details> </Detector> <BugPattern type="ANDROID_LOG_CALL_DETECTED"> <ShortDescription>android.util.Log usage detected !</ShortDescription> <LongDescription>android.util.Log usage detected !</LongDescription> <Details> <![CDATA[ <p> Please not use android.util.Log ! Use 'ru.sberbank.mobile.core.log.Logger' instead. ]]> </Details> </BugPattern> <BugCode abbrev="ANDR_LOG">android.util.Log usage detected !</BugCode> </MessageCollection> Custom Detector Code
Recall that the detector works with bytecode. To understand the detector's work, let us present a part of the bytecode of the onCreate () Activity method, where Lod.d is called (“aaa”, “bbb”);
javap -c LoginActivity.class Compiled from "LoginActivity.java" public class ru.sbrf.testandroidapp.LoginActivity extends android.support.v7.app.AppCompatActivity implements android.app.LoaderManager$LoaderCallbacks<android.database.Cursor> { public ru.sbrf.testandroidapp.LoginActivity(); protected void onCreate(android.os.Bundle); Code: 0: aload_0 1: aload_1 2: invokespecial #10 // Method android/support/v7/app/AppCompatActivity.onCreate:(Landroid/os/Bundle;)V 5: ldc #11 // String aaa 7: ldc #12 // String bbb 9: invokestatic #13 // Method android/util/Log.d:(Ljava/lang/String;Ljava/lang/String;)I 12: pop 13: aload_0 14: ldc #15 // int 2130968603 16: invokevirtual #16 // Method setContentView:(I)V Line 9 will search for our detector. This is a static method call in the android.util.Log class.
Detector code
public class AndroidLogCallDetector extends BytecodeScanningDetector implements StatelessDetector { public static final String BUG_TYPE = "ANDROID_LOG_CALL_DETECTED"; public static final String ANDROID_UTIL_LOG = "android/util/Log"; public static final String ALLOWED_LOGER_CLASS = "ru.sberbank.mobile.core.log.Logger"; private final BugReporter bugReporter; private String clsName; public AndroidLogCallDetector(BugReporter bugReporter) { this.bugReporter = bugReporter; } @Override public void visitClassContext(ClassContext classContext) { JavaClass cls = classContext.getJavaClass(); clsName = cls.getClassName(); //skip allowed logger class if (!ALLOWED_LOGER_CLASS.equals(clsName)) { super.visitClassContext(classContext); } } @Override public void sawOpcode(int seen) { if (seen == Const.INVOKESTATIC && ANDROID_UTIL_LOG.equals(getClassConstantOperand())) { BugInstance bug = new BugInstance(this, BUG_TYPE, NORMAL_PRIORITY).addClassAndMethod(this) .addSourceLine(this); bugReporter.reportBug(bug); } } } We assemble the plugin:
./gradlew jar Testing
Copy the file of the plugin detector-1.0-SNAPSHOT.jar in /spotbugs-4.0.0-SNAPSHOT/plugin. Run ui interface spotbugs /spotbugs-4.0.0-SNAPSHOT/bin/spotbugs. Select the bytecode directory of the test android project for verification. / TestAndroidApp / app / build / intermediates / classes / debug
As you can see, there were all calls to the android.util.Log class. Now our custom detector can be used in the CI system. He will inform the developer about the bugs found.
Code Style Check
Now we will free the reviewer from checking the code for compliance with the code-style. For this we will use the Checkstyle plugin . To him, by the way, is attached a list of all the checks that have already been implemented, is here .
Plugin is configured using the checkstyle.xml configuration file. It indicates the necessary checks and their parameters. We give examples of the file for the following checks:
- java doc for public methods
- class order (fields, methods)
- variable modifier order
- presence of magic numbers
- binding {} in if
- default branch switch
- hashcode and equals pair and covariance
<i><?</i>xml version="1.0"<i>?></i> <!DOCTYPE module PUBLIC "Check Configuration" "http://checkstyle.sourceforge.net/dtds/configuration_1_3.dtd"<i>></i> <module name="Checker"> <property name="charset" value="UTF-8" /> <property name="severity" value="warning" /> <module name="TreeWalker"> <module name="EqualsHashCode" /> <module name="CovariantEquals" /> <module name="MagicNumber" /> <module name="DeclarationOrder" /> <module name="JavadocMethod"> <property name="scope" value="public" /> <property name="allowMissingParamTags" value="true" /> <property name="allowMissingThrowsTags" value="true" /> <property name="allowMissingReturnTag" value="true" /> <property name="allowThrowsTagsForSubclasses" value="true" /> </module> <module name="LineLength"> <property name="max" value="160" /> <property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://" /> </module> <module name="EmptyBlock"> <property name="option" value="TEXT" /> <property name="tokens" value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH" /> </module> <module name="NeedBraces" /> <module name="MissingSwitchDefault" /> <module name="ModifierOrder" /> <module name="OverloadMethodsDeclarationOrder" /> </module> </module> A set of ready-made rules and the ability to configure them allow you to check almost any requirements. But if you need to create your own special rule, this can also be done according to the instructions .
Note that Checkstyle works with the source code of the project, the analysis of which was done using ANTLR . To implement the verification, you must implement the visitor, which is traversed through the syntax tree of the source code. In this article we will not consider the creation of a custom code-style check.
Add checkstyle plugin to android application
Add lines to build.gradle
apply plugin: 'checkstyle' task checkstyle(type: Checkstyle) { configFile file("$project.rootDir/app/checkstyle.xml") source 'src/main/java' include '**/*.java' exclude '**/gen/**' exclude '**/R.java' exclude '**/BuildConfig.java' classpath = files() } tasks.withType(Checkstyle) { reports { xml.enabled false html.enabled true } } Run the check ./gradlew checkstyle.
The report will be in the /build/reports/checkstyle/checkstyle.html directory. Here is an example report.
But the code that was checked
20 static public double getCount(){ 21 if(true) Log.<i>i</i>("a","b"); 22 return 43.2*15.0; 23 } So, we have freed revisers from the routine work - checking the code-style and checking the project agreements. Now developers will have more time for creativity, and the code will become better. We hope the material was useful, we will be happy to answer your questions in the comments.
Source: https://habr.com/ru/post/342634/
All Articles