Merge branch 'more-fixes' into 'master'

A few Java fixes and tweaks, enable more PMD rules



See merge request !266

CONTRIBUTING.md

@@ -58,6 +58,8 @@ To get all the logcat messages by F-Droid, you can run:
 * Use gradle with `--daemon` if you are going to build F-Droid multiple times.
 * If you get a message like `Could not find com.android.support:support-...`,
   make sure that you have the latest Android support maven repository.
+* When building as part of AOSP with `Android.mk`, make sure you have a
+  recent version of Gradle installed as `gradlew` will not be used.
 
 ## Running the test suite
 

app/build.gradle

@@ -208,16 +208,8 @@ pmd {
 }
 
 task pmd(type: Pmd, dependsOn: assembleDebug) {
-    ruleSets = [
+    ruleSetFiles = files("${project.rootDir}/config/pmd/rules.xml")
-        //'java-basic',
+    ruleSets = [] // otherwise defaults clash with the list in rules.xml
-        'java-unusedcode',
-        'java-android',
-        'java-clone',
-        'java-finalizers',
-        'java-imports',
-        'java-migrating',
-        //'java-unnecessary', // too nitpicky with parenthesis
-    ]
     source 'src/main/java', 'src/test/java', 'src/androidTest/java'
     include '**/*.java'
 }

app/src/androidTest/java/mock/MockFDroidResources.java

@@ -7,7 +7,7 @@ import org.fdroid.fdroid.R;
 
 public class MockFDroidResources extends MockResources {
 
-    private Context getStringDelegatingContext;
+    private final Context getStringDelegatingContext;
 
     public MockFDroidResources(Context getStringDelegatingContext) {
         this.getStringDelegatingContext = getStringDelegatingContext;

app/src/androidTest/java/mock/MockInstallablePackageManager.java

@@ -10,7 +10,7 @@ import java.util.List;
 
 public class MockInstallablePackageManager extends MockPackageManager {
 
-    private List<PackageInfo> info = new ArrayList<>();
+    private final List<PackageInfo> info = new ArrayList<>();
 
     @Override
     public List<PackageInfo> getInstalledPackages(int flags) {

app/src/androidTest/java/org/fdroid/fdroid/FileCompatTest.java

@@ -30,14 +30,13 @@ public class FileCompatTest {
 
     private static final String TAG = "FileCompatTest";
 
-    private File dir;
     private SanitizedFile sourceFile;
     private SanitizedFile destFile;
 
     @Before
     public void setUp() {
         Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation();
-        dir = TestUtils.getWriteableDir(instrumentation);
+        File dir = TestUtils.getWriteableDir(instrumentation);
         sourceFile = SanitizedFile.knownSanitized(TestUtils.copyAssetToDir(instrumentation.getContext(), "simpleIndex.jar", dir));
         destFile = new SanitizedFile(dir, "dest-" + UUID.randomUUID() + ".testproduct");
         assertFalse(destFile.exists());

app/src/androidTest/java/org/fdroid/fdroid/MultiRepoUpdaterTest.java

@@ -160,7 +160,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase {
      * repository indeed contains the apks that it said it would provide.
      */
     private void assertExpected() {
-        Log.d(TAG, "Asserting all versions of each .apk are in index.");
+        Log.i(TAG, "Asserting all versions of each .apk are in index.");
         List<Repo> repos = RepoProvider.Helper.all(context);
         assertEquals("Repos", 3, repos.size());
 
@@ -173,7 +173,7 @@ public class MultiRepoUpdaterTest extends InstrumentationTestCase {
      *
      */
     private void assertSomewhatAcceptable() {
-        Log.d(TAG, "Asserting at least one versions of each .apk is in index.");
+        Log.i(TAG, "Asserting at least one versions of each .apk is in index.");
         List<Repo> repos = RepoProvider.Helper.all(context);
         assertEquals("Repos", 3, repos.size());
 

app/src/androidTest/java/org/fdroid/fdroid/TestUtils.java

@@ -200,7 +200,7 @@ public class TestUtils {
         OutputStream output = null;
         try {
             tempFile = File.createTempFile(assetName + "-", ".testasset", directory);
-            Log.d(TAG, "Copying asset file " + assetName + " to directory " + directory);
+            Log.i(TAG, "Copying asset file " + assetName + " to directory " + directory);
             input = context.getAssets().open(assetName);
             output = new FileOutputStream(tempFile);
             Utils.copy(input, output);

app/src/main/java/com/google/zxing/encode/QRCodeEncoder.java

@@ -37,7 +37,7 @@ public final class QRCodeEncoder {
     private static final int WHITE = 0xFFFFFFFF;
     private static final int BLACK = 0xFF000000;
 
-    private int dimension = Integer.MIN_VALUE;
+    private final int dimension;
     private String contents;
     private String displayContents;
     private String title;

app/src/main/java/org/fdroid/fdroid/AndroidXMLDecompress.java

@@ -68,7 +68,7 @@ import java.util.zip.ZipFile;
  * @see <a href="https://stackoverflow.com/a/4761689">a binary XML parser</a>
  */
 public class AndroidXMLDecompress {
-    public static int startTag = 0x00100102;
+    public static final int START_TAG = 0x00100102;
 
     /**
      * Just get the XML attributes from the {@code <manifest>} element.
@@ -82,7 +82,7 @@ public class AndroidXMLDecompress {
         int stringTableOffset = stringIndexTableOffset + numbStrings * 4;
         int xmlTagOffset = littleEndianWord(binaryXml, 3 * 4);
         for (int i = xmlTagOffset; i < binaryXml.length - 4; i += 4) {
-            if (littleEndianWord(binaryXml, i) == startTag) {
+            if (littleEndianWord(binaryXml, i) == START_TAG) {
                 xmlTagOffset = i;
                 break;
             }
@@ -92,11 +92,11 @@ public class AndroidXMLDecompress {
         while (offset < binaryXml.length) {
             int tag0 = littleEndianWord(binaryXml, offset);
 
-            if (tag0 == startTag) {
+            if (tag0 == START_TAG) {
                 int numbAttrs = littleEndianWord(binaryXml, offset + 7 * 4);
                 offset += 9 * 4;
 
-                HashMap<String, Object> attributes = new HashMap<String, Object>(3);
+                HashMap<String, Object> attributes = new HashMap<>(3);
                 for (int i = 0; i < numbAttrs; i++) {
                     int attributeNameStringIndex = littleEndianWord(binaryXml, offset + 1 * 4);
                     int attributeValueStringIndex = littleEndianWord(binaryXml, offset + 2 * 4);
@@ -117,7 +117,7 @@ public class AndroidXMLDecompress {
             // we only need the first <manifest> start tag
             break;
         }
-        return new HashMap<String, Object>(0);
+        return new HashMap<>(0);
     }
 
     public static byte[] getManifestFromFilename(String filename) throws IOException {
@@ -137,9 +137,7 @@ public class AndroidXMLDecompress {
         is.read(buf);
 
         is.close();
-        if (zip != null) {
+        zip.close();
-            zip.close();
-        }
         return buf;
     }
 

app/src/main/java/org/fdroid/fdroid/AppDetails.java

@@ -439,7 +439,9 @@ public class AppDetails extends AppCompatActivity {
      */
     private void cleanUpFinishedDownload() {
         activeDownloadUrlString = null;
-        headerFragment.removeProgress();
+        if (headerFragment != null) {
+            headerFragment.removeProgress();
+        }
         unregisterDownloaderReceivers();
     }
 
@@ -464,6 +466,9 @@ public class AppDetails extends AppCompatActivity {
     }
 
     private void unregisterDownloaderReceivers() {
+        if (localBroadcastManager == null) {
+            return;
+        }
         localBroadcastManager.unregisterReceiver(startedReceiver);
         localBroadcastManager.unregisterReceiver(progressReceiver);
         localBroadcastManager.unregisterReceiver(completeReceiver);
@@ -863,8 +868,7 @@ public class AppDetails extends AppCompatActivity {
     }
 
     private void startDownload(Apk apk, String repoAddress) {
-        String urlString = Utils.getApkUrl(repoAddress, apk);
+        activeDownloadUrlString = Utils.getApkUrl(repoAddress, apk);
-        activeDownloadUrlString = urlString;
         registerDownloaderReceivers();
         headerFragment.startProgress();
         DownloaderService.queue(this, apk.packageName, activeDownloadUrlString);

app/src/main/java/org/fdroid/fdroid/RepoXMLHandler.java

@@ -268,7 +268,7 @@ public class RepoXMLHandler extends DefaultHandler {
         curchars.setLength(0);
     }
 
-    private String cleanWhiteSpace(@Nullable String str) {
+    private static String cleanWhiteSpace(@Nullable String str) {
         return str == null ? null : str.replaceAll("\\s", " ");
     }
 }

app/src/main/java/org/fdroid/fdroid/UpdateService.java

@@ -489,15 +489,18 @@ public class UpdateService extends IntentService implements ProgressListener {
                         AppProvider.DataColumns.PACKAGE_NAME,
                         AppProvider.DataColumns.SUGGESTED_VERSION_CODE,
                 }, null, null, null);
-        cursor.moveToFirst();
+        if (cursor != null) {
-        for (int i = 0; i < cursor.getCount(); i++) {
+            cursor.moveToFirst();
-            App app = new App(cursor);
+            for (int i = 0; i < cursor.getCount(); i++) {
-            Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVersionCode, new String[]{
+                App app = new App(cursor);
-                    ApkProvider.DataColumns.NAME,
+                Apk apk = ApkProvider.Helper.find(this, app.packageName, app.suggestedVersionCode, new String[]{
-            });
+                        ApkProvider.DataColumns.NAME,
-            String urlString = Utils.getApkUrl(repoAddress, apk);
+                });
-            DownloaderService.queue(this, app.packageName, urlString);
+                String urlString = Utils.getApkUrl(repoAddress, apk);
-            cursor.moveToNext();
+                DownloaderService.queue(this, app.packageName, urlString);
+                cursor.moveToNext();
+            }
+            cursor.close();
         }
     }
 

app/src/main/java/org/fdroid/fdroid/data/AppProvider.java

@@ -572,7 +572,7 @@ public class AppProvider extends FDroidProvider {
     public static Uri getSearchUri(Repo repo, String query) {
         return getContentUri().buildUpon()
             .appendPath(PATH_SEARCH_REPO)
-            .appendPath(repo.id + "")
+            .appendPath(String.valueOf(repo.id))
             .appendPath(query)
             .build();
     }

app/src/main/java/org/fdroid/fdroid/data/DBHelper.java

@@ -116,80 +116,82 @@ class DBHelper extends SQLiteOpenHelper {
     }
 
     private void populateRepoNames(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 37) {
+        if (oldVersion >= 37) {
-            Utils.debugLog(TAG, "Populating repo names from the url");
+            return;
-            final String[] columns = {"address", "_id"};
+        }
-            Cursor cursor = db.query(TABLE_REPO, columns,
+        Utils.debugLog(TAG, "Populating repo names from the url");
-                    "name IS NULL OR name = ''", null, null, null, null);
+        final String[] columns = {"address", "_id"};
-            if (cursor != null) {
+        Cursor cursor = db.query(TABLE_REPO, columns,
-                if (cursor.getCount() > 0) {
+                "name IS NULL OR name = ''", null, null, null, null);
-                    cursor.moveToFirst();
+        if (cursor != null) {
-                    while (!cursor.isAfterLast()) {
+            if (cursor.getCount() > 0) {
-                        String address = cursor.getString(0);
+                cursor.moveToFirst();
-                        long id = cursor.getInt(1);
+                while (!cursor.isAfterLast()) {
-                        ContentValues values = new ContentValues(1);
+                    String address = cursor.getString(0);
-                        String name = Repo.addressToName(address);
+                    long id = cursor.getInt(1);
-                        values.put("name", name);
+                    ContentValues values = new ContentValues(1);
-                        final String[] args = {Long.toString(id)};
+                    String name = Repo.addressToName(address);
-                        Utils.debugLog(TAG, "Setting repo name to '" + name + "' for repo " + address);
+                    values.put("name", name);
-                        db.update(TABLE_REPO, values, "_id = ?", args);
+                    final String[] args = {Long.toString(id)};
-                        cursor.moveToNext();
+                    Utils.debugLog(TAG, "Setting repo name to '" + name + "' for repo " + address);
-                    }
+                    db.update(TABLE_REPO, values, "_id = ?", args);
+                    cursor.moveToNext();
                 }
-                cursor.close();
             }
+            cursor.close();
         }
     }
 
     private void renameRepoId(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 36 && !columnExists(db, TABLE_REPO, "_id")) {
+        if (oldVersion >= 36 || columnExists(db, TABLE_REPO, "_id")) {
-
+            return;
-            Utils.debugLog(TAG, "Renaming " + TABLE_REPO + ".id to _id");
-            db.beginTransaction();
-
-            try {
-                // http://stackoverflow.com/questions/805363/how-do-i-rename-a-column-in-a-sqlite-database-table#805508
-                String tempTableName = TABLE_REPO + "__temp__";
-                db.execSQL("ALTER TABLE " + TABLE_REPO + " RENAME TO " + tempTableName + ";");
-
-                // I realise this is available in the CREATE_TABLE_REPO above,
-                // however I have a feeling that it will need to be the same as the
-                // current structure of the table as of DBVersion 36, or else we may
-                // get into strife. For example, if there was a field that
-                // got removed, then it will break the "insert select"
-                // statement. Therefore, I've put a copy of CREATE_TABLE_REPO
-                // here that is the same as it was at DBVersion 36.
-                String createTableDdl = "create table " + TABLE_REPO + " ("
-                        + "_id integer not null primary key, "
-                        + "address text not null, "
-                        + "name text, "
-                        + "description text, "
-                        + "inuse integer not null, "
-                        + "priority integer not null, "
-                        + "pubkey text, "
-                        + "fingerprint text, "
-                        + "maxage integer not null default 0, "
-                        + "version integer not null default 0, "
-                        + "lastetag text, "
-                        + "lastUpdated string);";
-
-                db.execSQL(createTableDdl);
-
-                String nonIdFields = "address, name, description, inuse, priority, " +
-                        "pubkey, fingerprint, maxage, version, lastetag, lastUpdated";
-
-                String insertSql = "INSERT INTO " + TABLE_REPO +
-                        "(_id, " + nonIdFields + " ) " +
-                        "SELECT id, " + nonIdFields + " FROM " + tempTableName + ";";
-
-                db.execSQL(insertSql);
-                db.execSQL("DROP TABLE " + tempTableName + ";");
-                db.setTransactionSuccessful();
-            } catch (Exception e) {
-                Log.e(TAG, "Error renaming id to _id", e);
-            }
-            db.endTransaction();
         }
+
+        Utils.debugLog(TAG, "Renaming " + TABLE_REPO + ".id to _id");
+        db.beginTransaction();
+
+        try {
+            // http://stackoverflow.com/questions/805363/how-do-i-rename-a-column-in-a-sqlite-database-table#805508
+            String tempTableName = TABLE_REPO + "__temp__";
+            db.execSQL("ALTER TABLE " + TABLE_REPO + " RENAME TO " + tempTableName + ";");
+
+            // I realise this is available in the CREATE_TABLE_REPO above,
+            // however I have a feeling that it will need to be the same as the
+            // current structure of the table as of DBVersion 36, or else we may
+            // get into strife. For example, if there was a field that
+            // got removed, then it will break the "insert select"
+            // statement. Therefore, I've put a copy of CREATE_TABLE_REPO
+            // here that is the same as it was at DBVersion 36.
+            String createTableDdl = "create table " + TABLE_REPO + " ("
+                    + "_id integer not null primary key, "
+                    + "address text not null, "
+                    + "name text, "
+                    + "description text, "
+                    + "inuse integer not null, "
+                    + "priority integer not null, "
+                    + "pubkey text, "
+                    + "fingerprint text, "
+                    + "maxage integer not null default 0, "
+                    + "version integer not null default 0, "
+                    + "lastetag text, "
+                    + "lastUpdated string);";
+
+            db.execSQL(createTableDdl);
+
+            String nonIdFields = "address, name, description, inuse, priority, " +
+                    "pubkey, fingerprint, maxage, version, lastetag, lastUpdated";
+
+            String insertSql = "INSERT INTO " + TABLE_REPO +
+                    "(_id, " + nonIdFields + " ) " +
+                    "SELECT id, " + nonIdFields + " FROM " + tempTableName + ";";
+
+            db.execSQL(insertSql);
+            db.execSQL("DROP TABLE " + tempTableName + ";");
+            db.setTransactionSuccessful();
+        } catch (Exception e) {
+            Log.e(TAG, "Error renaming id to _id", e);
+        }
+        db.endTransaction();
     }
 
     @Override
@@ -299,36 +301,37 @@ class DBHelper extends SQLiteOpenHelper {
      * key in sqlite - table must be recreated).
      */
     private void migrateRepoTable(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 20) {
+        if (oldVersion >= 20) {
-            List<Repo> oldrepos = new ArrayList<>();
+            return;
-            Cursor cursor = db.query(TABLE_REPO,
+        }
-                    new String[] {"address", "inuse", "pubkey"},
+        List<Repo> oldrepos = new ArrayList<>();
-                    null, null, null, null, null);
+        Cursor cursor = db.query(TABLE_REPO,
-            if (cursor != null) {
+                new String[] {"address", "inuse", "pubkey"},
-                if (cursor.getCount() > 0) {
+                null, null, null, null, null);
-                    cursor.moveToFirst();
+        if (cursor != null) {
-                    while (!cursor.isAfterLast()) {
+            if (cursor.getCount() > 0) {
-                        Repo repo = new Repo();
+                cursor.moveToFirst();
-                        repo.address = cursor.getString(0);
+                while (!cursor.isAfterLast()) {
-                        repo.inuse = cursor.getInt(1) == 1;
+                    Repo repo = new Repo();
-                        repo.signingCertificate = cursor.getString(2);
+                    repo.address = cursor.getString(0);
-                        oldrepos.add(repo);
+                    repo.inuse = cursor.getInt(1) == 1;
-                        cursor.moveToNext();
+                    repo.signingCertificate = cursor.getString(2);
-                    }
+                    oldrepos.add(repo);
+                    cursor.moveToNext();
                 }
-                cursor.close();
-            }
-            db.execSQL("drop table " + TABLE_REPO);
-            db.execSQL(CREATE_TABLE_REPO);
-            for (final Repo repo : oldrepos) {
-                ContentValues values = new ContentValues();
-                values.put("address", repo.address);
-                values.put("inuse", repo.inuse);
-                values.put("priority", 10);
-                values.put("pubkey", repo.signingCertificate);
-                values.put("lastetag", (String) null);
-                db.insert(TABLE_REPO, null, values);
             }
+            cursor.close();
+        }
+        db.execSQL("drop table " + TABLE_REPO);
+        db.execSQL(CREATE_TABLE_REPO);
+        for (final Repo repo : oldrepos) {
+            ContentValues values = new ContentValues();
+            values.put("address", repo.address);
+            values.put("inuse", repo.inuse);
+            values.put("priority", 10);
+            values.put("pubkey", repo.signingCertificate);
+            values.put("lastetag", (String) null);
+            db.insert(TABLE_REPO, null, values);
         }
     }
 
@@ -350,22 +353,23 @@ class DBHelper extends SQLiteOpenHelper {
     private void addNameAndDescriptionToRepo(SQLiteDatabase db, int oldVersion) {
         boolean nameExists = columnExists(db, TABLE_REPO, "name");
         boolean descriptionExists = columnExists(db, TABLE_REPO, "description");
-        if (oldVersion < 21 && !(nameExists && descriptionExists)) {
+        if (oldVersion >= 21 || (nameExists && descriptionExists)) {
-            if (!nameExists) {
+            return;
-                db.execSQL("alter table " + TABLE_REPO + " add column name text");
-            }
-            if (!descriptionExists) {
-                db.execSQL("alter table " + TABLE_REPO + " add column description text");
-            }
-            insertNameAndDescription(db, R.string.fdroid_repo_address,
-                    R.string.fdroid_repo_name, R.string.fdroid_repo_description);
-            insertNameAndDescription(db, R.string.fdroid_archive_address,
-                    R.string.fdroid_archive_name, R.string.fdroid_archive_description);
-            insertNameAndDescription(db, R.string.guardianproject_repo_address,
-                    R.string.guardianproject_repo_name, R.string.guardianproject_repo_description);
-            insertNameAndDescription(db, R.string.guardianproject_archive_address,
-                    R.string.guardianproject_archive_name, R.string.guardianproject_archive_description);
         }
+        if (!nameExists) {
+            db.execSQL("alter table " + TABLE_REPO + " add column name text");
+        }
+        if (!descriptionExists) {
+            db.execSQL("alter table " + TABLE_REPO + " add column description text");
+        }
+        insertNameAndDescription(db, R.string.fdroid_repo_address,
+                R.string.fdroid_repo_name, R.string.fdroid_repo_description);
+        insertNameAndDescription(db, R.string.fdroid_archive_address,
+                R.string.fdroid_archive_name, R.string.fdroid_archive_description);
+        insertNameAndDescription(db, R.string.guardianproject_repo_address,
+                R.string.guardianproject_repo_name, R.string.guardianproject_repo_description);
+        insertNameAndDescription(db, R.string.guardianproject_archive_address,
+                R.string.guardianproject_archive_name, R.string.guardianproject_archive_description);
 
     }
 
@@ -374,115 +378,128 @@ class DBHelper extends SQLiteOpenHelper {
      * calculate its fingerprint and save it to the database.
      */
     private void addFingerprintToRepo(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 44) {
+        if (oldVersion >= 44) {
-            if (!columnExists(db, TABLE_REPO, "fingerprint")) {
+            return;
-                db.execSQL("alter table " + TABLE_REPO + " add column fingerprint text");
+        }
-            }
+        if (!columnExists(db, TABLE_REPO, "fingerprint")) {
-            List<Repo> oldrepos = new ArrayList<>();
+            db.execSQL("alter table " + TABLE_REPO + " add column fingerprint text");
-            Cursor cursor = db.query(TABLE_REPO,
+        }
-                    new String[] {"address", "pubkey"},
+        List<Repo> oldrepos = new ArrayList<>();
-                    null, null, null, null, null);
+        Cursor cursor = db.query(TABLE_REPO,
-            if (cursor != null) {
+                new String[] {"address", "pubkey"},
-                if (cursor.getCount() > 0) {
+                null, null, null, null, null);
-                    cursor.moveToFirst();
+        if (cursor != null) {
-                    while (!cursor.isAfterLast()) {
+            if (cursor.getCount() > 0) {
-                        Repo repo = new Repo();
+                cursor.moveToFirst();
-                        repo.address = cursor.getString(0);
+                while (!cursor.isAfterLast()) {
-                        repo.signingCertificate = cursor.getString(1);
+                    Repo repo = new Repo();
-                        oldrepos.add(repo);
+                    repo.address = cursor.getString(0);
-                        cursor.moveToNext();
+                    repo.signingCertificate = cursor.getString(1);
-                    }
+                    oldrepos.add(repo);
+                    cursor.moveToNext();
                 }
-                cursor.close();
-            }
-            for (final Repo repo : oldrepos) {
-                ContentValues values = new ContentValues();
-                values.put("fingerprint", Utils.calcFingerprint(repo.signingCertificate));
-                db.update(TABLE_REPO, values, "address = ?", new String[] {repo.address});
             }
+            cursor.close();
+        }
+        for (final Repo repo : oldrepos) {
+            ContentValues values = new ContentValues();
+            values.put("fingerprint", Utils.calcFingerprint(repo.signingCertificate));
+            db.update(TABLE_REPO, values, "address = ?", new String[] {repo.address});
         }
     }
 
     private void addMaxAgeToRepo(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 30 && !columnExists(db, TABLE_REPO, "maxage")) {
+        if (oldVersion >= 30 || columnExists(db, TABLE_REPO, "maxage")) {
-            db.execSQL("alter table " + TABLE_REPO + " add column maxage integer not null default 0");
+            return;
         }
+        db.execSQL("alter table " + TABLE_REPO + " add column maxage integer not null default 0");
     }
 
     private void addVersionToRepo(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 33 && !columnExists(db, TABLE_REPO, "version")) {
+        if (oldVersion >= 33 || columnExists(db, TABLE_REPO, "version")) {
-            db.execSQL("alter table " + TABLE_REPO + " add column version integer not null default 0");
+            return;
         }
+        db.execSQL("alter table " + TABLE_REPO + " add column version integer not null default 0");
     }
 
     private void addLastUpdatedToRepo(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 35 && !columnExists(db, TABLE_REPO, "lastUpdated")) {
+        if (oldVersion >= 35 || columnExists(db, TABLE_REPO, "lastUpdated")) {
-            Utils.debugLog(TAG, "Adding lastUpdated column to " + TABLE_REPO);
+            return;
-            db.execSQL("Alter table " + TABLE_REPO + " add column lastUpdated string");
         }
+        Utils.debugLog(TAG, "Adding lastUpdated column to " + TABLE_REPO);
+        db.execSQL("Alter table " + TABLE_REPO + " add column lastUpdated string");
     }
 
     private void addIsSwapToRepo(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 47 && !columnExists(db, TABLE_REPO, "isSwap")) {
+        if (oldVersion >= 47 || columnExists(db, TABLE_REPO, "isSwap")) {
-            Utils.debugLog(TAG, "Adding isSwap field to " + TABLE_REPO + " table in db.");
+            return;
-            db.execSQL("alter table " + TABLE_REPO + " add column isSwap boolean default 0;");
         }
+        Utils.debugLog(TAG, "Adding isSwap field to " + TABLE_REPO + " table in db.");
+        db.execSQL("alter table " + TABLE_REPO + " add column isSwap boolean default 0;");
     }
 
     private void addCredentialsToRepo(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 52) {
+        if (oldVersion >= 52) {
-            if (!columnExists(db, TABLE_REPO, "username")) {
+            return;
-                Utils.debugLog(TAG, "Adding username field to " + TABLE_REPO + " table in db.");
+        }
-                db.execSQL("alter table " + TABLE_REPO + " add column username string;");
+        if (!columnExists(db, TABLE_REPO, "username")) {
-            }
+            Utils.debugLog(TAG, "Adding username field to " + TABLE_REPO + " table in db.");
+            db.execSQL("alter table " + TABLE_REPO + " add column username string;");
+        }
 
-            if (!columnExists(db, TABLE_REPO, "password")) {
+        if (!columnExists(db, TABLE_REPO, "password")) {
-                Utils.debugLog(TAG, "Adding password field to " + TABLE_REPO + " table in db.");
+            Utils.debugLog(TAG, "Adding password field to " + TABLE_REPO + " table in db.");
-                db.execSQL("alter table " + TABLE_REPO + " add column password string;");
+            db.execSQL("alter table " + TABLE_REPO + " add column password string;");
-            }
         }
     }
 
     private void addChangelogToApp(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 48 && !columnExists(db, TABLE_APP, "changelogURL")) {
+        if (oldVersion >= 48 || columnExists(db, TABLE_APP, "changelogURL")) {
-            Utils.debugLog(TAG, "Adding changelogURL column to " + TABLE_APP);
+            return;
-            db.execSQL("alter table " + TABLE_APP + " add column changelogURL text");
         }
+        Utils.debugLog(TAG, "Adding changelogURL column to " + TABLE_APP);
+        db.execSQL("alter table " + TABLE_APP + " add column changelogURL text");
     }
 
     private void addIconUrlLargeToApp(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 49 && !columnExists(db, TABLE_APP, "iconUrlLarge")) {
+        if (oldVersion >= 49 || columnExists(db, TABLE_APP, "iconUrlLarge")) {
-            Utils.debugLog(TAG, "Adding iconUrlLarge columns to " + TABLE_APP);
+            return;
-            db.execSQL("alter table " + TABLE_APP + " add column iconUrlLarge text");
         }
+        Utils.debugLog(TAG, "Adding iconUrlLarge columns to " + TABLE_APP);
+        db.execSQL("alter table " + TABLE_APP + " add column iconUrlLarge text");
     }
 
     private void updateIconUrlLarge(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 50) {
+        if (oldVersion >= 50) {
-            Utils.debugLog(TAG, "Recalculating app icon URLs so that the newly added large icons will get updated.");
+            return;
-            AppProvider.UpgradeHelper.updateIconUrls(context, db);
-            clearRepoEtags(db);
         }
+        Utils.debugLog(TAG, "Recalculating app icon URLs so that the newly added large icons will get updated.");
+        AppProvider.UpgradeHelper.updateIconUrls(context, db);
+        clearRepoEtags(db);
     }
 
     private void addAuthorToApp(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 53 && !columnExists(db, TABLE_APP, "author")) {
+        if (oldVersion >= 53) {
+            return;
+        }
+        if (!columnExists(db, TABLE_APP, "author")) {
             Utils.debugLog(TAG, "Adding author column to " + TABLE_APP);
             db.execSQL("alter table " + TABLE_APP + " add column author text");
         }
-        if (oldVersion < 53 && !columnExists(db, TABLE_APP, "email")) {
+        if (!columnExists(db, TABLE_APP, "email")) {
             Utils.debugLog(TAG, "Adding email column to " + TABLE_APP);
             db.execSQL("alter table " + TABLE_APP + " add column email text");
         }
     }
 
     private void useMaxValueInMaxSdkVersion(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 54) {
+        if (oldVersion >= 54) {
-            Utils.debugLog(TAG, "Converting maxSdkVersion value 0 to " + Byte.MAX_VALUE);
+            return;
-            ContentValues values = new ContentValues();
-            values.put(ApkProvider.DataColumns.MAX_SDK_VERSION, Byte.MAX_VALUE);
-            db.update(TABLE_APK, values, ApkProvider.DataColumns.MAX_SDK_VERSION + " < 1", null);
         }
+        Utils.debugLog(TAG, "Converting maxSdkVersion value 0 to " + Byte.MAX_VALUE);
+        ContentValues values = new ContentValues();
+        values.put(ApkProvider.DataColumns.MAX_SDK_VERSION, Byte.MAX_VALUE);
+        db.update(TABLE_APK, values, ApkProvider.DataColumns.MAX_SDK_VERSION + " < 1", null);
     }
 
     /**
@@ -501,14 +518,15 @@ class DBHelper extends SQLiteOpenHelper {
         // was is specified by the user. We don't want to weely-neely nuke that data.
         // and the new way to deal with changes to the table structure is to add a
         // if (oldVersion < x && !columnExists(...) and then alter the table as required.
-        if (oldVersion < 42) {
+        if (oldVersion >= 42) {
-            context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit()
+            return;
-                    .putBoolean("triedEmptyUpdate", false).commit();
-            db.execSQL("drop table " + TABLE_APP);
-            db.execSQL("drop table " + TABLE_APK);
-            clearRepoEtags(db);
-            createAppApk(db);
         }
+        context.getSharedPreferences("FDroid", Context.MODE_PRIVATE).edit()
+                .putBoolean("triedEmptyUpdate", false).commit();
+        db.execSQL("drop table " + TABLE_APP);
+        db.execSQL("drop table " + TABLE_APK);
+        clearRepoEtags(db);
+        createAppApk(db);
     }
 
     private static void createAppApk(SQLiteDatabase db) {
@@ -527,10 +545,11 @@ class DBHelper extends SQLiteOpenHelper {
     // If any column was added or removed, just drop the table, create it
     // again and let the cache be filled from scratch again.
     private void recreateInstalledCache(SQLiteDatabase db, int oldVersion) {
-        if (oldVersion < 51) {
+        if (oldVersion >= 51) {
-            db.execSQL(DROP_TABLE_INSTALLED_APP);
+            return;
-            createInstalledApp(db);
         }
+        db.execSQL(DROP_TABLE_INSTALLED_APP);
+        createInstalledApp(db);
     }
 
     private static boolean columnExists(SQLiteDatabase db,

app/src/main/java/org/fdroid/fdroid/data/RepoPersister.java

@@ -8,7 +8,6 @@ import android.net.Uri;
 import android.os.RemoteException;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
-import android.util.Log;
 
 import org.fdroid.fdroid.CompatibilityChecker;
 import org.fdroid.fdroid.RepoUpdater;
@@ -99,7 +98,7 @@ public class RepoPersister {
         }
 
         if (apksToSave.size() > 0 || appsToSave.size() > 0) {
-            Log.d(TAG, "Flushing details of up to " + MAX_APP_BUFFER + " apps and their packages to the database.");
+            Utils.debugLog(TAG, "Flushing details of up to " + MAX_APP_BUFFER + " apps and their packages to the database.");
             flushAppsToDbInBatch();
             flushApksToDbInBatch();
             apksToSave.clear();

app/src/main/java/org/fdroid/fdroid/installer/ApkSignatureVerifier.java

@@ -24,8 +24,8 @@ import android.content.Context;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.Signature;
-import android.util.Log;
 
+import org.fdroid.fdroid.Utils;
 import org.spongycastle.util.encoders.Hex;
 
 import java.io.ByteArrayOutputStream;
@@ -58,9 +58,9 @@ public class ApkSignatureVerifier {
             return true;
         }
 
-        Log.d(TAG, "Signature mismatch!");
+        Utils.debugLog(TAG, "Signature mismatch!");
-        Log.d(TAG, "APK sig: " + Hex.toHexString(getApkSignature(apkFile)));
+        Utils.debugLog(TAG, "APK sig: " + Hex.toHexString(getApkSignature(apkFile)));
-        Log.d(TAG, "F-Droid sig: " + Hex.toHexString(getFDroidSignature()));
+        Utils.debugLog(TAG, "F-Droid sig: " + Hex.toHexString(getFDroidSignature()));
         return false;
     }
 

app/src/main/java/org/fdroid/fdroid/installer/Installer.java

@@ -162,10 +162,7 @@ public abstract class Installer {
             return false;
         }
         Hasher hasher = new Hasher(hashType, apkFile);
-        if (hasher != null && hasher.match(hash)) {
+        return hasher.match(hash);
-            return true;
-        }
-        return false;
     }
 
     /**

app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java

@@ -76,7 +76,7 @@ public class PrivilegedInstaller extends Installer {
     private static final String PRIVILEGED_EXTENSION_SERVICE_INTENT = "org.fdroid.fdroid.privileged.IPrivilegedService";
     public static final String PRIVILEGED_EXTENSION_PACKAGE_NAME = "org.fdroid.fdroid.privileged";
 
-    private Activity mActivity;
+    private final Activity mActivity;
 
     private static final int REQUEST_CONFIRM_PERMS = 0;
 

app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java

@@ -91,10 +91,6 @@ public class SwapService extends Service {
     @NonNull
     private final Set<String> appsToSwap = new HashSet<>();
 
-    public SwapService() {
-        super();
-    }
-
     /**
      * Where relevant, the state of the swap process will be saved to disk using preferences.
      * Note that this is not always useful, for example saving the "current wifi network" is

app/src/main/java/org/fdroid/fdroid/localrepo/type/BluetoothSwap.java

@@ -22,7 +22,6 @@ public final class BluetoothSwap extends SwapType {
 
     @NonNull
     private final BluetoothAdapter adapter;
-    private BroadcastReceiver receiver;
     private boolean isDiscoverable;
 
     @Nullable
@@ -64,7 +63,7 @@ public final class BluetoothSwap extends SwapType {
             return;
         }
 
-        receiver = new BroadcastReceiver() {
+        BroadcastReceiver receiver = new BroadcastReceiver() {
             @Override
             public void onReceive(Context context, Intent intent) {
                 switch (intent.getIntExtra(BluetoothAdapter.EXTRA_SCAN_MODE, -1)) {

app/src/main/java/org/fdroid/fdroid/net/DownloaderService.java

@@ -94,7 +94,7 @@ public class DownloaderService extends Service {
     private static volatile Downloader downloader;
     private LocalBroadcastManager localBroadcastManager;
 
-    private static final HashMap<String, Integer> QUEUE_WHATS = new HashMap<String, Integer>();
+    private static final HashMap<String, Integer> QUEUE_WHATS = new HashMap<>();
     private int what;
 
     private final class ServiceHandler extends Handler {

app/src/main/java/org/fdroid/fdroid/net/LocalHTTPD.java

@@ -221,7 +221,7 @@ public class LocalHTTPD extends NanoHTTPD {
         try {
             // Calculate etag
             String etag = Integer
-                    .toHexString((file.getAbsolutePath() + file.lastModified() + "" + file.length())
+                    .toHexString((file.getAbsolutePath() + file.lastModified() + String.valueOf(file.length()))
                             .hashCode());
 
             // Support (simple) skipping:
@@ -268,7 +268,7 @@ public class LocalHTTPD extends NanoHTTPD {
                     fis.skip(startFrom);
 
                     res = createResponse(Response.Status.PARTIAL_CONTENT, mime, fis);
-                    res.addHeader("Content-Length", "" + dataLen);
+                    res.addHeader("Content-Length", String.valueOf(dataLen));
                     res.addHeader("Content-Range", "bytes " + startFrom + "-" + endAt + "/"
                             + fileLen);
                     res.addHeader("ETag", etag);
@@ -278,7 +278,7 @@ public class LocalHTTPD extends NanoHTTPD {
                     res = createResponse(Response.Status.NOT_MODIFIED, mime, "");
                 } else {
                     res = createResponse(Response.Status.OK, mime, new FileInputStream(file));
-                    res.addHeader("Content-Length", "" + fileLen);
+                    res.addHeader("Content-Length", String.valueOf(fileLen));
                     res.addHeader("ETag", etag);
                 }
             }

app/src/main/java/org/fdroid/fdroid/net/bluetooth/BluetoothServer.java

@@ -259,7 +259,7 @@ public class BluetoothServer extends Thread {
             try {
                 // Calculate etag
                 String etag = Integer
-                        .toHexString((file.getAbsolutePath() + file.lastModified() + "" + file.length())
+                        .toHexString((file.getAbsolutePath() + file.lastModified() + String.valueOf(file.length()))
                                 .hashCode());
 
                 // Support (simple) skipping:
@@ -306,7 +306,7 @@ public class BluetoothServer extends Thread {
                         fis.skip(startFrom);
 
                         res = createResponse(NanoHTTPD.Response.Status.PARTIAL_CONTENT, mime, fis);
-                        res.addHeader("Content-Length", "" + dataLen);
+                        res.addHeader("Content-Length", String.valueOf(dataLen));
                         res.addHeader("Content-Range", "bytes " + startFrom + "-" + endAt + "/"
                                 + fileLen);
                         res.addHeader("ETag", etag);
@@ -316,7 +316,7 @@ public class BluetoothServer extends Thread {
                         res = createResponse(NanoHTTPD.Response.Status.NOT_MODIFIED, mime, "");
                     } else {
                         res = createResponse(NanoHTTPD.Response.Status.OK, mime, new FileInputStream(file));
-                        res.addHeader("Content-Length", "" + fileLen);
+                        res.addHeader("Content-Length", String.valueOf(fileLen));
                         res.addHeader("ETag", etag);
                     }
                 }

app/src/main/java/org/fdroid/fdroid/privileged/install/InstallExtensionDialogActivity.java

@@ -328,9 +328,6 @@ public class InstallExtensionDialogActivity extends FragmentActivity {
      * 3. Verify that install worked
      */
     private void postInstall() {
-        // hack to get theme applied (which is not automatically applied due to activity's Theme.NoDisplay
-        ContextThemeWrapper theme = new ContextThemeWrapper(this, FDroidApp.getCurThemeResId());
-
         int isInstalledCorrectly =
                 PrivilegedInstaller.isExtensionInstalledCorrectly(this);
 
@@ -367,6 +364,9 @@ public class InstallExtensionDialogActivity extends FragmentActivity {
                 throw new RuntimeException("unhandled return");
         }
 
+        // hack to get theme applied (which is not automatically applied due to activity's Theme.NoDisplay
+        ContextThemeWrapper theme = new ContextThemeWrapper(this, FDroidApp.getCurThemeResId());
+
         AlertDialog.Builder builder = new AlertDialog.Builder(theme)
                 .setTitle(title)
                 .setMessage(message)

app/src/main/java/org/fdroid/fdroid/privileged/views/AppSecurityPermissions.java

@@ -420,16 +420,14 @@ public class AppSecurityPermissions {
         final int base = pInfo.protectionLevel & PermissionInfo.PROTECTION_MASK_BASE;
         final boolean isNormal = base == PermissionInfo.PROTECTION_NORMAL;
         final boolean isDangerous = base == PermissionInfo.PROTECTION_DANGEROUS;
-        final boolean wasGranted =
-                (existingReqFlags & PackageInfo.REQUESTED_PERMISSION_GRANTED) != 0;
 
         // Dangerous and normal permissions are always shown to the user
         if (isNormal || isDangerous) {
             return true;
         }
 
-        final boolean isDevelopment =
+        final boolean isDevelopment = (pInfo.protectionLevel & PermissionInfo.PROTECTION_FLAG_DEVELOPMENT) != 0;
-                (pInfo.protectionLevel & PermissionInfo.PROTECTION_FLAG_DEVELOPMENT) != 0;
+        final boolean wasGranted = (existingReqFlags & PackageInfo.REQUESTED_PERMISSION_GRANTED) != 0;
 
         // Development permissions are only shown to the user if they are already
         // granted to the app -- if we are installing an app and they are not

app/src/main/java/org/fdroid/fdroid/privileged/views/CaffeinatedScrollView.java

@@ -40,6 +40,7 @@ public class CaffeinatedScrollView extends ScrollView {
     /**
      * Make this visible so we can call it
      */
+    @SuppressWarnings("PMD.UselessOverridingMethod")
     @Override
     public boolean awakenScrollBars() {
         return super.awakenScrollBars();

config/pmd/rules.xml

@@ -0,0 +1,30 @@
+<?xml version="1.0"?>
+<ruleset name="Custom ruleset"
+        xmlns="http://pmd.sf.net/ruleset/1.0.0"
+        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">
+
+    <!--<rule ref="rulesets/java/basic.xml"/>-->
+    <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/imports.xml"/>
+    <rule ref="rulesets/java/migrating.xml"/>
+    <rule ref="rulesets/java/unnecessary.xml">
+        <exclude name="UselessParentheses"/> <!--Too nitpicky-->
+    </rule>
+    <rule ref="rulesets/java/optimizations.xml/PrematureDeclaration"/>
+    <rule ref="rulesets/java/optimizations.xml/AddEmptyString"/>
+    <rule ref="rulesets/java/controversial.xml/UnnecessaryConstructor"/>
+    <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"/>
+</ruleset>