Merge pull request #4767 from nextcloud/app-code-checker

Check language files and database schema with app code checker

7 files changed, 246 insertions, 13 deletions

diff --git a/.drone.yml b/.drone.yml
index f26e5d3f870..11458a30398 100644
--- a/.drone.yml
+++ b/.drone.yml
@@ -15,18 +15,7 @@ pipeline:
   checkers:
     image: nextcloudci/php7.0:php7.0-7
     commands:
-      - bash ./build/autoloaderchecker.sh
-      - bash ./build/mergejschecker.sh
-      - php ./build/translation-checker.php
-      - php ./build/htaccess-checker.php
-      - ./occ app:check-code admin_audit
-      - ./occ app:check-code comments
-      - ./occ app:check-code federation
-      - ./occ app:check-code sharebymail
-      - ./occ app:check-code systemtags
-      - ./occ app:check-code theming
-      - ./occ app:check-code workflowengine
-      - php ./build/signed-off-checker.php
+      - ./autotest-checkers.sh
     when:
       matrix:
         TESTS: checkers
diff --git a/autotest-checkers.sh b/autotest-checkers.sh
new file mode 100755
index 00000000000..35c945a17e9
--- /dev/null
+++ b/autotest-checkers.sh
@@ -0,0 +1,43 @@
+#!/usr/bin/env bash
+#
+RESULT=0
+
+bash ./build/autoloaderchecker.sh
+RESULT=$(($RESULT+$?))
+bash ./build/mergejschecker.sh
+RESULT=$(($RESULT+$?))
+php ./build/translation-checker.php
+RESULT=$(($RESULT+$?))
+php ./build/htaccess-checker.php
+RESULT=$(($RESULT+$?))
+
+
+for app in $(find "apps/" -mindepth 1 -maxdepth 1 -type d -exec basename {} \;); do
+    echo "Testing $app"
+    if
+        [ "$app" == "dav" ] || \
+        [ "$app" == "encryption" ] || \
+        [ "$app" == "federatedfilesharing" ] || \
+        [ "$app" == "files" ] || \
+        [ "$app" == "files_external" ] || \
+        [ "$app" == "files_sharing" ] || \
+        [ "$app" == "files_trashbin" ] || \
+        [ "$app" == "files_versions" ] || \
+        [ "$app" == "lookup_server_connector" ] || \
+        [ "$app" == "provisioning_api" ] || \
+        [ "$app" == "testing" ] || \
+        [ "$app" == "twofactor_backupcodes" ] || \
+        [ "$app" == "updatenotification" ] || \
+        [ "$app" == "user_ldap" ]
+    then
+        ./occ app:check-code --skip-checkers "$app"
+    else
+        ./occ app:check-code "$app"
+    fi
+    RESULT=$(($RESULT+$?))
+done;
+
+php ./build/signed-off-checker.php
+RESULT=$(($RESULT+$?))
+
+exit $RESULT
diff --git a/core/Command/App/CheckCode.php b/core/Command/App/CheckCode.php
index aa618b26cec..46b9b748ada 100644
--- a/core/Command/App/CheckCode.php
+++ b/core/Command/App/CheckCode.php
@@ -26,8 +26,10 @@
 namespace OC\Core\Command\App;
 
 use OC\App\CodeChecker\CodeChecker;
+use OC\App\CodeChecker\DatabaseSchemaChecker;
 use OC\App\CodeChecker\EmptyCheck;
 use OC\App\CodeChecker\InfoChecker;
+use OC\App\CodeChecker\LanguageParseChecker;
 use OC\App\InfoParser;
 use Stecman\Component\Symfony\Console\BashCompletion\Completion\CompletionAwareInterface;
 use Stecman\Component\Symfony\Console\BashCompletion\CompletionContext;
@@ -70,6 +72,12 @@ class CheckCode extends Command implements CompletionAwareInterface  {
 				[ 'private', 'deprecation', 'strong-comparison' ]
 			)
 			->addOption(
+				'--skip-checkers',
+				null,
+				InputOption::VALUE_NONE,
+				'skips the the code checkers to only check info.xml, language and database schema'
+			)
+			->addOption(
 				'--skip-validate-info',
 				null,
 				InputOption::VALUE_NONE,
@@ -117,7 +125,10 @@ class CheckCode extends Command implements CompletionAwareInterface  {
 				$output->writeln("    <error>line $line: {$p['disallowedToken']} - {$p['reason']}</error>");
 			}
 		});
-		$errors = $codeChecker->analyse($appId);
+		$errors = [];
+		if(!$input->getOption('skip-checkers')) {
+			$errors = $codeChecker->analyse($appId);
+		}
 
 		if(!$input->getOption('skip-validate-info')) {
 			$infoChecker = new InfoChecker($this->infoParser);
@@ -171,6 +182,27 @@ class CheckCode extends Command implements CompletionAwareInterface  {
 			$infoErrors = $infoChecker->analyse($appId);
 
 			$errors = array_merge($errors, $infoErrors);
+
+			$languageParser = new LanguageParseChecker();
+			$languageErrors = $languageParser->analyse($appId);
+
+			foreach ($languageErrors as $languageError) {
+				$output->writeln("<error>$languageError</error>");
+			}
+
+			$errors = array_merge($errors, $languageErrors);
+
+			$databaseSchema = new DatabaseSchemaChecker();
+			$schemaErrors = $databaseSchema->analyse($appId);
+
+			foreach ($schemaErrors['errors'] as $schemaError) {
+				$output->writeln("<error>$schemaError</error>");
+			}
+			foreach ($schemaErrors['warnings'] as $schemaWarning) {
+				$output->writeln("<comment>$schemaWarning</comment>");
+			}
+
+			$errors = array_merge($errors, $schemaErrors['errors']);
 		}
 
 		$this->analyseUpdateFile($appId, $output);
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 59cac3db775..10233f07a2a 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -332,10 +332,12 @@ return array(
     'OC\\App\\AppStore\\Version\\VersionParser' => $baseDir . '/lib/private/App/AppStore/Version/VersionParser.php',
     'OC\\App\\CodeChecker\\AbstractCheck' => $baseDir . '/lib/private/App/CodeChecker/AbstractCheck.php',
     'OC\\App\\CodeChecker\\CodeChecker' => $baseDir . '/lib/private/App/CodeChecker/CodeChecker.php',
+    'OC\\App\\CodeChecker\\DatabaseSchemaChecker' => $baseDir . '/lib/private/App/CodeChecker/DatabaseSchemaChecker.php',
     'OC\\App\\CodeChecker\\DeprecationCheck' => $baseDir . '/lib/private/App/CodeChecker/DeprecationCheck.php',
     'OC\\App\\CodeChecker\\EmptyCheck' => $baseDir . '/lib/private/App/CodeChecker/EmptyCheck.php',
     'OC\\App\\CodeChecker\\ICheck' => $baseDir . '/lib/private/App/CodeChecker/ICheck.php',
     'OC\\App\\CodeChecker\\InfoChecker' => $baseDir . '/lib/private/App/CodeChecker/InfoChecker.php',
+    'OC\\App\\CodeChecker\\LanguageParseChecker' => $baseDir . '/lib/private/App/CodeChecker/LanguageParseChecker.php',
     'OC\\App\\CodeChecker\\NodeVisitor' => $baseDir . '/lib/private/App/CodeChecker/NodeVisitor.php',
     'OC\\App\\CodeChecker\\PrivateCheck' => $baseDir . '/lib/private/App/CodeChecker/PrivateCheck.php',
     'OC\\App\\CodeChecker\\StrongComparisonCheck' => $baseDir . '/lib/private/App/CodeChecker/StrongComparisonCheck.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index b7e584c324a..9c0f0871133 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -362,10 +362,12 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\App\\AppStore\\Version\\VersionParser' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/Version/VersionParser.php',
         'OC\\App\\CodeChecker\\AbstractCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/AbstractCheck.php',
         'OC\\App\\CodeChecker\\CodeChecker' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/CodeChecker.php',
+        'OC\\App\\CodeChecker\\DatabaseSchemaChecker' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/DatabaseSchemaChecker.php',
         'OC\\App\\CodeChecker\\DeprecationCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/DeprecationCheck.php',
         'OC\\App\\CodeChecker\\EmptyCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/EmptyCheck.php',
         'OC\\App\\CodeChecker\\ICheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/ICheck.php',
         'OC\\App\\CodeChecker\\InfoChecker' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/InfoChecker.php',
+        'OC\\App\\CodeChecker\\LanguageParseChecker' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/LanguageParseChecker.php',
         'OC\\App\\CodeChecker\\NodeVisitor' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/NodeVisitor.php',
         'OC\\App\\CodeChecker\\PrivateCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/PrivateCheck.php',
         'OC\\App\\CodeChecker\\StrongComparisonCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/StrongComparisonCheck.php',
diff --git a/lib/private/App/CodeChecker/DatabaseSchemaChecker.php b/lib/private/App/CodeChecker/DatabaseSchemaChecker.php
new file mode 100644
index 00000000000..1255dec25c1
--- /dev/null
+++ b/lib/private/App/CodeChecker/DatabaseSchemaChecker.php
@@ -0,0 +1,105 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017, Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\App\CodeChecker;
+
+class DatabaseSchemaChecker {
+
+	/**
+	 * @param string $appId
+	 * @return array
+	 */
+	public function analyse($appId) {
+		$appPath = \OC_App::getAppPath($appId);
+		if ($appPath === false) {
+			throw new \RuntimeException("No app with given id <$appId> known.");
+		}
+
+		if (!file_exists($appPath . '/appinfo/database.xml')) {
+			return ['errors' => [], 'warnings' => []];
+		}
+
+		libxml_use_internal_errors(true);
+		$loadEntities = libxml_disable_entity_loader(false);
+		$xml = simplexml_load_file($appPath . '/appinfo/database.xml');
+		libxml_disable_entity_loader($loadEntities);
+
+
+		$errors = $warnings = [];
+
+		foreach ($xml->table as $table) {
+			// Table names
+			if (strpos($table->name, '*dbprefix*') !== 0) {
+				$errors[] = 'Database schema error: name of table ' . $table->name . ' does not start with *dbprefix*';
+			}
+			$tableName = substr($table->name, strlen('*dbprefix*'));
+			if (strpos($tableName, '*dbprefix*') !== false) {
+				$warnings[] = 'Database schema warning: *dbprefix* should only appear once in name of table ' . $table->name;
+			}
+
+			if (strlen($tableName) > 27) {
+				$errors[] = 'Database schema error: Name of table ' . $table->name . ' is too long (' . strlen($tableName) . '), max. 27 characters (21 characters for tables with autoincrement) + *dbprefix* allowed';
+			}
+
+			$hasAutoIncrement = false;
+
+			// Column names
+			foreach ($table->declaration->field as $column) {
+				if (strpos($column->name, '*dbprefix*') !== false) {
+					$warnings[] = 'Database schema warning: *dbprefix* should not appear in name of column ' . $column->name . ' on table ' . $table->name;
+				}
+
+				if (strlen($column->name) > 30) {
+					$errors[] = 'Database schema error: Name of column ' . $column->name . ' on table ' . $table->name . ' is too long (' . strlen($tableName) . '), max. 30 characters allowed';
+				}
+
+				if ($column->autoincrement) {
+					if ($hasAutoIncrement) {
+						$errors[] = 'Database schema error: Table ' . $table->name . ' has multiple autoincrement columns';
+					}
+
+					if (strlen($tableName) > 21) {
+						$errors[] = 'Database schema error: Name of table ' . $table->name . ' is too long (' . strlen($tableName) . '), max. 27 characters (21 characters for tables with autoincrement) + *dbprefix* allowed';
+					}
+
+					$hasAutoIncrement = true;
+				}
+			}
+
+			// Index names
+			foreach ($table->declaration->index as $index) {
+				$hasPrefix = strpos($index->name, '*dbprefix*');
+				if ($hasPrefix !== false && $hasPrefix !== 0) {
+					$warnings[] = 'Database schema warning: *dbprefix* should only appear at the beginning in name of index ' . $index->name . ' on table ' . $table->name;
+				}
+
+				$indexName = $hasPrefix === 0 ? substr($index->name, strlen('*dbprefix*')) : $index->name;
+				if (strlen($indexName) > 27) {
+					$errors[] = 'Database schema error: Name of index ' . $index->name . ' on table ' . $table->name . ' is too long (' . strlen($tableName) . '), max. 27 characters + *dbprefix* allowed';
+				}
+			}
+		}
+
+		return ['errors' => $errors, 'warnings' => $warnings];
+	}
+}
diff --git a/lib/private/App/CodeChecker/LanguageParseChecker.php b/lib/private/App/CodeChecker/LanguageParseChecker.php
new file mode 100644
index 00000000000..35354869339
--- /dev/null
+++ b/lib/private/App/CodeChecker/LanguageParseChecker.php
@@ -0,0 +1,60 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017, Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\App\CodeChecker;
+
+class LanguageParseChecker {
+
+	/**
+	 * @param string $appId
+	 * @return array
+	 */
+	public function analyse($appId) {
+		$appPath = \OC_App::getAppPath($appId);
+		if ($appPath === false) {
+			throw new \RuntimeException("No app with given id <$appId> known.");
+		}
+
+		if (!is_dir($appPath . '/l10n/')) {
+			return [];
+		}
+
+		$errors = [];
+		$directory = new \DirectoryIterator($appPath . '/l10n/');
+
+		foreach ($directory as $file) {
+			if ($file->getExtension() !== 'json') {
+				continue;
+			}
+
+			$content = file_get_contents($file->getPathname());
+			json_decode($content, true);
+
+			if (json_last_error() !== JSON_ERROR_NONE) {
+				$errors[] = 'Invalid language file found: l10n/' . $file->getFilename() . ': ' . json_last_error_msg();
+			}
+		}
+
+		return $errors;
+	}
+}