Merge branch 'modernize-pmd-and-randoms' into 'master'

modernize PMD and random fixes

See merge request fdroid/fdroidclient!861
This commit is contained in:
Hans-Christoph Steiner 2020-01-10 16:40:22 +00:00
commit 92327cbc99
23 changed files with 121 additions and 106 deletions

View File

@ -40,13 +40,13 @@ test_lint_pmd_checkstyle:
- sed -i -e 's,textReport .*,textReport true,' app/build.gradle
- ./gradlew testFullDebugUnitTest
- ./gradlew lint
- ./gradlew pmd || export EXITVALUE=1
- ./gradlew checkstyle || export EXITVALUE=1
- ./tools/check-format-strings.py || export EXITVALUE=1
- ./tools/check-fastlane-whitespace.py || export EXITVALUE=1
- ./tools/remove-unused-and-blank-translations.py || export EXITVALUE=1
- ./gradlew pmd || (export EXITVALUE=1; echo ERROR !!)
- ./gradlew checkstyle || (export EXITVALUE=1; echo ERROR !!)
- ./tools/check-format-strings.py || (export EXITVALUE=1; echo ERROR !!)
- ./tools/check-fastlane-whitespace.py || (export EXITVALUE=1; echo ERROR !!)
- ./tools/remove-unused-and-blank-translations.py || (export EXITVALUE=1; echo ERROR !!)
- echo "These are unused or blank translations that should be removed:"
- git --no-pager diff --ignore-all-space --name-only --exit-code app/src/*/res/values*/strings.xml || export EXITVALUE=1
- git --no-pager diff --ignore-all-space --name-only --exit-code app/src/*/res/values*/strings.xml || (export EXITVALUE=1; echo ERROR !!)
- exit $EXITVALUE
errorprone:

View File

@ -2,7 +2,7 @@ apply plugin: 'com.android.application'
apply plugin: 'checkstyle'
apply plugin: 'pmd'
/* gets the version name from the latest Git tag, stripping the leading v off */
/* gets the version name from the latest Git tag */
def getVersionName = { ->
def stdout = new ByteArrayOutputStream()
exec {
@ -192,7 +192,7 @@ task checkstyle(type: Checkstyle) {
}
pmd {
toolVersion = '5.5.1'
toolVersion = '6.20.0'
consoleOutput = true
}
@ -202,7 +202,6 @@ task pmdMain(type: Pmd) {
ruleSets = [] // otherwise defaults clash with the list in rules.xml
source 'src/main/java'
include '**/*.java'
exclude '**/kellinwood/**/*.java'
}
task pmdTest(type: Pmd) {

View File

@ -123,7 +123,7 @@ public class Netstat {
try {
BufferedReader in = new BufferedReader(new FileReader("/proc/net/tcp"));
String line;
while ((line = in.readLine()) != null) {
while ((line = in.readLine()) != null) { // NOPMD
Matcher matcher = NET_PATTERN.matcher(line);
if (matcher.find()) {
final Connection c = new Connection();
@ -156,7 +156,7 @@ public class Netstat {
try {
BufferedReader in = new BufferedReader(new FileReader("/proc/net/udp"));
String line;
while ((line = in.readLine()) != null) {
while ((line = in.readLine()) != null) { // NOPMD
Matcher matcher = NET_PATTERN.matcher(line);
if (matcher.find()) {
final Connection c = new Connection();
@ -189,7 +189,7 @@ public class Netstat {
try {
BufferedReader in = new BufferedReader(new FileReader("/proc/net/raw"));
String line;
while ((line = in.readLine()) != null) {
while ((line = in.readLine()) != null) { // NOPMD
Matcher matcher = NET_PATTERN.matcher(line);
if (matcher.find()) {
final Connection c = new Connection();

View File

@ -68,7 +68,7 @@ public class Hasher {
InputStream input = null;
try {
input = new BufferedInputStream(new FileInputStream(file));
while ((read = input.read(buffer)) > 0) {
while ((read = input.read(buffer)) > 0) { // NOPMD Avoid assignments in operands
digest.update(buffer, 0, read);
}
} catch (Exception e) {

View File

@ -199,7 +199,7 @@ class NotificationHelper {
Notification notification;
if (updates.size() != 1 || useStackedNotifications()) {
if (updates.size() == 0) {
if (updates.isEmpty()) {
// No updates, remove summary
notificationManager.cancel(GROUP_UPDATES, NOTIFY_ID_UPDATES);
} else {
@ -208,7 +208,7 @@ class NotificationHelper {
}
}
if (installed.size() != 1 || useStackedNotifications()) {
if (installed.size() == 0) {
if (installed.isEmpty()) {
// No installed, remove summary
notificationManager.cancel(GROUP_INSTALLED, NOTIFY_ID_INSTALLED);
} else {

View File

@ -64,8 +64,8 @@ public class Provisioner {
List<ProvisionPlaintext> plaintexts = p.extractProvisionsPlaintext(files);
List<Provision> provisions = p.parseProvisions(plaintexts);
if (provisions == null || provisions.size() == 0) {
Utils.debugLog(TAG, "Provision dir does not contain any provisions: '" + provisionDir.getAbsolutePath() + "' moving on ...");
if (provisions == null || provisions.isEmpty()) {
Utils.debugLog(TAG, "Provision dir is empty: '" + provisionDir.getAbsolutePath() + "' moving on ...");
} else {
int cleanupCounter = 0;
for (Provision provision : provisions) {
@ -173,7 +173,7 @@ public class Provisioner {
try {
in = new ZipInputStream(new FileInputStream(file));
ZipEntry zipEntry;
while ((zipEntry = in.getNextEntry()) != null) {
while ((zipEntry = in.getNextEntry()) != null) { // NOPMD Avoid assignments in operands
String name = zipEntry.getName();
if ("repo_provision.json".equals(name)) {
if (plain.getRepositoryProvision() != null) {

View File

@ -524,7 +524,7 @@ public final class Utils {
byte[] dataBytes = new byte[8192];
int nread;
while ((nread = bis.read(dataBytes)) != -1) {
while ((nread = bis.read(dataBytes)) != -1) { // NOPMD Avoid assignments in operands
md.update(dataBytes, 0, nread);
}

View File

@ -638,7 +638,7 @@ public class AppProvider extends FDroidProvider {
// Put in a Set to remove duplicates
final Set<String> keywordSet = new HashSet<>(Arrays.asList(query.split("\\s")));
if (keywordSet.size() == 0) {
if (keywordSet.isEmpty()) {
return new AppQuerySelection();
}

View File

@ -27,7 +27,7 @@ public abstract class FDroidProvider extends ContentProvider {
static final int CODE_LIST = 1;
static final int CODE_SINGLE = 2;
private boolean isApplyingBatch;
private boolean applyingBatch;
protected abstract String getTableName();
@ -47,7 +47,7 @@ public abstract class FDroidProvider extends ContentProvider {
* Based on http://stackoverflow.com/a/15886915.
*/
protected final boolean isApplyingBatch() {
return this.isApplyingBatch;
return this.applyingBatch;
}
@NonNull
@ -55,7 +55,7 @@ public abstract class FDroidProvider extends ContentProvider {
public ContentProviderResult[] applyBatch(@NonNull ArrayList<ContentProviderOperation> operations)
throws OperationApplicationException {
ContentProviderResult[] result = null;
isApplyingBatch = true;
applyingBatch = true;
final SQLiteDatabase db = db();
db.beginTransaction();
try {
@ -63,7 +63,7 @@ public abstract class FDroidProvider extends ContentProvider {
db.setTransactionSuccessful();
} finally {
db.endTransaction();
isApplyingBatch = false;
applyingBatch = false;
}
return result;
}

View File

@ -143,7 +143,7 @@ abstract class QueryBuilder {
}
private String orderBySql() {
if (orderBys.size() == 0) {
if (orderBys.isEmpty()) {
return "";
}
return " ORDER BY " + TextUtils.join(", ", orderBys);

View File

@ -182,12 +182,12 @@ public class RepoPersister {
private void calcApkCompatibilityFlags(List<Apk> apks) {
for (final Apk apk : apks) {
final List<String> reasons = checker.getIncompatibleReasons(apk);
if (reasons.size() > 0) {
apk.compatible = false;
apk.incompatibleReasons = reasons.toArray(new String[reasons.size()]);
} else {
if (reasons.isEmpty()) {
apk.compatible = true;
apk.incompatibleReasons = null;
} else {
apk.compatible = false;
apk.incompatibleReasons = reasons.toArray(new String[reasons.size()]);
}
}
}

View File

@ -80,7 +80,7 @@ public class RepoProvider extends FDroidProvider {
boolean haveTriedWithoutPath = false;
while (repo == null && !haveTriedWithoutPath) {
if (pathSegments.size() == 0) {
if (pathSegments.isEmpty()) {
haveTriedWithoutPath = true;
} else {
pathSegments.remove(pathSegments.size() - 1);
@ -99,7 +99,11 @@ public class RepoProvider extends FDroidProvider {
String address, String[] projection) {
List<Repo> repos = findBy(
context, Cols.ADDRESS, address, projection);
return repos.size() > 0 ? repos.get(0) : null;
if (repos.isEmpty()) {
return null;
} else {
return repos.get(0);
}
}
public static List<Repo> all(Context context) {

View File

@ -153,13 +153,13 @@ public class AppDetailsRecyclerViewAdapter
addItem(VIEWTYPE_DONATE);
addItem(VIEWTYPE_LINKS);
addItem(VIEWTYPE_PERMISSIONS);
if (versions.size() > 0) {
if (versions.isEmpty()) {
addItem(VIEWTYPE_NO_VERSIONS);
} else {
addItem(VIEWTYPE_VERSIONS);
if (showVersions) {
setShowVersions(true);
}
} else {
addItem(VIEWTYPE_NO_VERSIONS);
}
notifyDataSetChanged();
@ -563,7 +563,7 @@ public class AppDetailsRecyclerViewAdapter
updateAntiFeaturesWarning();
buttonPrimaryView.setText(R.string.menu_install);
buttonPrimaryView.setVisibility(versions.size() > 0 ? View.VISIBLE : View.GONE);
buttonPrimaryView.setVisibility(versions.isEmpty() ? View.GONE : View.VISIBLE);
buttonSecondaryView.setText(R.string.menu_uninstall);
buttonSecondaryView.setVisibility(app.isUninstallable(context) ? View.VISIBLE : View.INVISIBLE);
buttonSecondaryView.setOnClickListener(new View.OnClickListener() {

View File

@ -21,7 +21,7 @@ public class AppListItemState {
private int progressMax = -1;
private boolean showInstallButton;
private boolean showCheckBox;
private boolean isCheckBoxChecked;
private boolean checkBoxChecked;
public AppListItemState(@NonNull App app) {
this.app = app;
@ -124,7 +124,7 @@ public class AppListItemState {
}
public boolean isCheckBoxChecked() {
return isCheckBoxChecked;
return checkBoxChecked;
}
/**
@ -133,7 +133,7 @@ public class AppListItemState {
*/
public AppListItemState setCheckBoxStatus(boolean checked) {
this.showCheckBox = true;
this.isCheckBoxChecked = checked;
this.checkBoxChecked = checked;
return this;
}

View File

@ -9,7 +9,7 @@ import android.app.Activity;
* org.fdroid.fdroid.views.updates.UpdatesAdapter#delegatesManager}
* to specify a data type more specific than just {@link Object}.
*/
public abstract class AppUpdateData {
public abstract class AppUpdateData { // NOPMD This abstract class does not have any abstract methods
public final Activity activity;
public AppUpdateData(Activity activity) {

View File

@ -9,7 +9,7 @@ import org.robolectric.RuntimeEnvironment;
import org.robolectric.Shadows;
import org.robolectric.shadows.ShadowContentResolver;
public abstract class FDroidProviderTest {
public abstract class FDroidProviderTest { // NOPMD This abstract class does not have any abstract methods
protected ShadowContentResolver contentResolver;
protected ContextWrapper context;

View File

@ -840,7 +840,7 @@ public class RepoXMLHandlerTest {
for (App app : apps) {
if (expectedAntiFeatures.containsKey(app.packageName)) {
List<String> antiFeatures = expectedAntiFeatures.get(app.packageName);
if (antiFeatures.size() == 0) {
if (antiFeatures.isEmpty()) {
assertNull(app.antiFeatures);
} else {
List<String> actualAntiFeatures = new ArrayList<>();

View File

@ -423,7 +423,7 @@ public class IndexV1UpdaterTest extends FDroidProviderTest {
for (String field : allowed) {
fields.remove(field);
}
if (fields.size() > 0) {
if (!fields.isEmpty()) {
String sb = String.valueOf(instance.getClass()) + " has fields not setup for Jackson: " +
TextUtils.join(", ", fields) + "\nRead class javadoc for more info.";
fail(sb);

View File

@ -35,7 +35,7 @@ public class Issue763MultiRepo extends MultiIndexUpdaterTest {
}
@Test
public void antoxRepo() throws IndexUpdater.UpdateException {
public void testAntoxRepo() throws IndexUpdater.UpdateException {
assertAntoxEmpty();
setEnabled(microGRepo, true);
updateAntox();
@ -47,7 +47,7 @@ public class Issue763MultiRepo extends MultiIndexUpdaterTest {
}
@Test
public void microGRepo() throws IndexUpdater.UpdateException {
public void testMicroGRepo() throws IndexUpdater.UpdateException {
assertMicroGEmpty();
setEnabled(microGRepo, true);
updateMicroG();

View File

@ -1,15 +1,17 @@
buildscript {
repositories {
mavenCentral()
maven { url 'https://maven.google.com/' } // :-| must be before jcenter()
jcenter()
jcenter() // download from jCenter as last resort https://blog.autsoft.hu/a-confusing-dependency
}
dependencies {
classpath 'com.android.tools.build:gradle:3.1.1'
classpath 'com.android.tools.build:gradle:3.1.4'
}
}
allprojects {
repositories {
mavenCentral()
maven { url 'https://maven.google.com/' } // :-| must be before jcenter()
jcenter()
jcenter() // download from jCenter as last resort https://blog.autsoft.hu/a-confusing-dependency
}
}

View File

@ -4,6 +4,10 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd"
xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd">
<description>
Rules for the project code aka "main".
</description>
<!-- TODO <rule ref="category/java/errorprone.xml/CloseResource"/> -->
<rule ref="rulesets/java/imports.xml" />
</ruleset>

View File

@ -4,8 +4,14 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd"
xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd">
<description>
Rules for the test harness code aka "androidTest" and "test".
</description>
<!-- ideally these would be promoted to rules.xml -->
<rule ref="category/java/bestpractices.xml/ConstantsInInterface"/>
<rule ref="category/java/bestpractices.xml/UseCollectionIsEmpty"/>
<rule ref="category/java/errorprone.xml/AvoidFieldNameMatchingMethodName"/>
<rule ref="category/java/errorprone.xml/AvoidFieldNameMatchingTypeName"/>
<rule ref="rulesets/java/imports.xml">
<exclude name="TooManyStaticImports" /><!-- Static imports are commonly used for JUnit tests -->
</rule>
</ruleset>

View File

@ -4,49 +4,49 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd"
xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd">
<description>
Rules for the whole project
</description>
<rule ref="rulesets/java/basic.xml">
<exclude name="CollapsibleIfStatements"/> <!--sometimes forces hard to read code-->
</rule>
<rule ref="rulesets/java/unusedcode.xml"/>
<rule ref="rulesets/java/android.xml"/>
<rule ref="rulesets/java/clone.xml"/>
<rule ref="rulesets/java/finalizers.xml"/>
<rule ref="rulesets/java/migrating.xml"/>
<rule ref="rulesets/java/unnecessary.xml">
<exclude name="UselessParentheses"/> <!--Too nitpicky-->
</rule>
<rule ref="rulesets/java/empty.xml">
<exclude name="EmptyCatchBlock"/> <!--Ignoring an exception-->
<exclude name="EmptyWhileStmt"/> <!--Can be useful sometimes-->
</rule>
<rule ref="rulesets/java/optimizations.xml/PrematureDeclaration"/>
<rule ref="rulesets/java/optimizations.xml/AddEmptyString"/>
<rule ref="rulesets/java/optimizations.xml/UseArraysAsList"/>
<rule ref="rulesets/java/optimizations.xml/UnnecessaryWrapperObjectCreation"/>
<rule ref="rulesets/java/controversial.xml/UnnecessaryConstructor"/>
<rule ref="rulesets/java/controversial.xml/UnnecessaryParentheses"/>
<rule ref="rulesets/java/strictexception.xml/AvoidRethrowingException"/>
<rule ref="rulesets/java/strictexception.xml/AvoidCatchingThrowable"/>
<rule ref="rulesets/java/strictexception.xml/AvoidCatchingNPE"/>
<rule ref="rulesets/java/strictexception.xml/DoNotExtendJavaLangError"/>
<rule ref="rulesets/java/strictexception.xml/DoNotThrowExceptionInFinally"/>
<rule ref="rulesets/java/strictexception.xml/AvoidThrowingNewInstanceOfSameException"/>
<rule ref="rulesets/java/strictexception.xml/AvoidThrowingNullPointerException"/>
<rule ref="rulesets/java/design.xml/FinalFieldCouldBeStatic"/>
<rule ref="rulesets/java/design.xml/CloseResource"/>
<rule ref="rulesets/java/design.xml/DefaultLabelNotLastInSwitchStmt"/>
<rule ref="rulesets/java/design.xml/UnnecessaryLocalBeforeReturn"/>
<rule ref="rulesets/java/design.xml/NonCaseLabelInSwitchStatement"/>
<rule ref="rulesets/java/design.xml/EqualsNull"/>
<rule ref="rulesets/java/design.xml/IdempotentOperations"/>
<rule ref="rulesets/java/design.xml/ImmutableField"/>
<rule ref="rulesets/java/design.xml/SingularField"/>
<rule ref="rulesets/java/design.xml/SimplifyBooleanReturns"/>
<rule ref="rulesets/java/design.xml/SimplifyBooleanExpressions"/>
<rule ref="rulesets/java/design.xml/LogicInversion"/>
<rule ref="category/java/bestpractices.xml/AbstractClassWithoutAbstractMethod"/>
<rule ref="category/java/bestpractices.xml/AvoidReassigningLoopVariables"/>
<rule ref="category/java/bestpractices.xml/DefaultLabelNotLastInSwitchStmt"/>
<rule ref="category/java/bestpractices.xml/MissingOverride"/>
<rule ref="category/java/bestpractices.xml/UnusedFormalParameter"/>
<rule ref="category/java/bestpractices.xml/UnusedLocalVariable"/>
<rule ref="category/java/bestpractices.xml/UnusedPrivateField"/>
<rule ref="category/java/bestpractices.xml/UnusedPrivateMethod"/>
<rule ref="category/java/codestyle.xml/UnnecessaryConstructor"/>
<rule ref="category/java/codestyle.xml/UnnecessaryLocalBeforeReturn"/>
<rule ref="category/java/design.xml/AvoidRethrowingException"/>
<rule ref="category/java/design.xml/AvoidThrowingNewInstanceOfSameException"/>
<rule ref="category/java/design.xml/AvoidThrowingNullPointerException"/>
<rule ref="category/java/design.xml/DoNotExtendJavaLangError"/>
<rule ref="category/java/design.xml/FinalFieldCouldBeStatic"/>
<rule ref="category/java/design.xml/ImmutableField"/>
<rule ref="category/java/design.xml/LogicInversion"/>
<rule ref="category/java/design.xml/SimplifyBooleanExpressions"/>
<rule ref="category/java/design.xml/SimplifyBooleanReturns"/>
<rule ref="category/java/design.xml/SingularField"/>
<rule ref="category/java/errorprone.xml/AssignmentInOperand"/>
<rule ref="category/java/errorprone.xml/AvoidAssertAsIdentifier"/>
<rule ref="category/java/errorprone.xml/AvoidBranchingStatementAsLastInLoop"/>
<rule ref="category/java/errorprone.xml/AvoidCallingFinalize"/>
<rule ref="category/java/errorprone.xml/AvoidCatchingNPE"/>
<rule ref="category/java/errorprone.xml/AvoidCatchingThrowable"/>
<rule ref="category/java/errorprone.xml/AvoidMultipleUnaryOperators"/>
<rule ref="category/java/errorprone.xml/AvoidUsingOctalValues"/>
<rule ref="category/java/errorprone.xml/BrokenNullCheck"/>
<rule ref="category/java/errorprone.xml/CallSuperFirst"/>
<rule ref="category/java/errorprone.xml/CallSuperLast"/>
<rule ref="category/java/errorprone.xml/DoNotHardCodeSDCard" />
<rule ref="category/java/errorprone.xml/DoNotThrowExceptionInFinally"/>
<rule ref="category/java/errorprone.xml/EqualsNull"/>
<rule ref="category/java/errorprone.xml/IdempotentOperations"/>
<rule ref="category/java/errorprone.xml/NonCaseLabelInSwitchStatement"/>
<rule ref="category/java/performance.xml/AddEmptyString"/>
<rule ref="category/java/performance.xml/UnnecessaryWrapperObjectCreation"/>
<rule ref="category/java/performance.xml/UseArraysAsList"/>
<rule ref="category/java/security.xml"/>
</ruleset>