Merge pull request #214 from nextcloud/techdebt/noid/coding-standard

Check coding standard

.github/workflows/lint.yml

@@ -0,0 +1,45 @@
+name: Lint
+
+on:
+  pull_request:
+  push:
+    branches:
+      - master
+      - stable*
+
+jobs:
+  php:
+    runs-on: ubuntu-latest
+
+    strategy:
+      matrix:
+        php-versions: ['7.2', '7.3', '7.4']
+
+    name: php${{ matrix.php-versions }}
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Set up php ${{ matrix.php-versions }}
+        uses: shivammathur/setup-php@v1
+        with:
+          php-version: ${{ matrix.php-versions }}
+          coverage: none
+
+      - name: Lint
+        run: composer run lint || ( echo 'Please run `composer run lint` and fix your code' && exit 1 )
+
+  php-cs-fixer:
+    name: php-cs check
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@master
+      - name: Set up php
+        uses: shivammathur/setup-php@master
+        with:
+          php-version: 7.4
+          coverage: none
+      - name: Install dependencies
+        run: composer i
+      - name: Run coding standards check
+        run: composer run cs:check || ( echo 'Please run `composer run cs:fix` to format your code' && exit 1 )

.gitignore

@@ -1,3 +1,2 @@
 # due to the different Travis-CI PHP versions we MUST NOT lock to specific versions
-composer.lock
 vendor

.php_cs.cache

@@ -0,0 +1 @@
+{"php":"7.4.7","version":"2.16.4:v2.16.4#1023c3458137ab052f6ff1e09621a721bfdeca13","indent":"\t","lineEnding":"\n","rules":{"encoding":true,"full_opening_tag":true,"blank_line_after_namespace":true,"braces":{"position_after_anonymous_constructs":"same","position_after_control_structures":"same","position_after_functions_and_oop_constructs":"same"},"class_definition":true,"constant_case":true,"elseif":true,"function_declaration":{"closure_function_spacing":"one"},"indentation_type":true,"line_ending":true,"lowercase_keywords":true,"no_break_comment":true,"no_closing_tag":true,"no_spaces_after_function_name":true,"no_spaces_inside_parenthesis":true,"no_trailing_whitespace":true,"no_trailing_whitespace_in_comment":true,"single_blank_line_at_eof":true,"single_class_element_per_statement":true,"single_import_per_statement":true,"single_line_after_imports":true,"switch_case_semicolon_to_colon":true,"switch_case_space":true,"visibility_required":{"elements":["property","method","const"]},"align_multiline_comment":true,"array_indentation":true,"array_syntax":{"syntax":"short"},"blank_line_after_opening_tag":true,"no_unused_imports":true},"hashes":{"lib\/Capabilities.php":2332335149,"lib\/Settings\/RegistrationSettings.php":1187727085,"lib\/Controller\/RegisterController.php":481295791,"lib\/Controller\/SettingsController.php":1057137794,"lib\/Controller\/ApiController.php":1056010743,"lib\/Util\/CoreBridge.php":2569539796,"lib\/Service\/MailService.php":4185218457,"lib\/Service\/RegistrationException.php":1966977026,"lib\/Service\/RegistrationService.php":1344864019,"lib\/AppInfo\/Application.php":1184851739,"lib\/Db\/Registration.php":3951088540,"lib\/Db\/RegistrationMapper.php":3012923546,"templates\/email.newuser_html.php":1706810427,"templates\/email.validate_plaintext.php":1751926117,"templates\/email.newuser_plaintext.php":1706810427,"templates\/admin.php":4102502608,"templates\/message.php":1916485661,"templates\/email.validate_html.php":2905336319,"templates\/domains.php":2396246270,"templates\/form.php":3602162854,"templates\/email.newuser.disabled_plaintext.php":4246922555,"templates\/email.newuser.disabled_html.php":3101190368,"templates\/register.php":3871034857,"tests\/bootstrap.php":2619470924,"tests\/Unit\/Controller\/ApiControllerTest.php":3215627675,"tests\/Integration\/Controller\/RegisterControllerTest.php":3551540727,"tests\/Integration\/Service\/RegistrationServiceTest.php":3309052170,"appinfo\/app.php":2364261601,"appinfo\/routes.php":2340075781}}

.php_cs.dist

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+require_once './vendor/autoload.php';
+
+use Nextcloud\CodingStandard\Config;
+
+$config = new Config();
+$config
+	->getFinder()
+	->ignoreVCSIgnored(true)
+	->notPath('build')
+	->notPath('l10n')
+	->notPath('src')
+	->notPath('vendor')
+	->in(__DIR__);
+return $config;

composer.json

@@ -1,6 +1,12 @@
 {
+	"scripts": {
+		"cs:check": "php-cs-fixer fix --dry-run --diff",
+		"cs:fix": "php-cs-fixer fix",
+		"lint": "find . -name \\*.php -not -path './vendor/*' -not -path './build/*' -not -path './tests/integration/vendor/*' -print0 | xargs -0 -n1 php -l"
+	},
 	"require-dev": {
 		"phpunit/phpunit": "^8.0",
-        "christophwurst/nextcloud_testing": "0.10.0"
+		"christophwurst/nextcloud_testing": "0.10.0",
+		"nextcloud/coding-standard": "^0.3.0"
 	}
 }

lib/AppInfo/Application.php

@@ -26,8 +26,7 @@ namespace OCA\Registration\AppInfo;
 use OCP\AppFramework\App;
 
 class Application extends App {
-
-	public function __construct(array $urlParams = array()) {
+	public function __construct(array $urlParams = []) {
 		parent::__construct('registration', $urlParams);
 
 		$container = $this->getContainer();
@@ -39,5 +38,4 @@ class Application extends App {
 			$container->registerCapability(\OCA\Registration\Capabilities::class);
 		}
 	}
-
 }

lib/Capabilities.php

@@ -23,7 +23,6 @@
 
 namespace OCA\Registration;
 
-use OCP\Capabilities\ICapability;
 use OCP\Capabilities\IPublicCapability;
 use OCP\IURLGenerator;
 
@@ -47,5 +46,4 @@ class Capabilities implements IPublicCapability {
 			]
 		];
 	}
-
 }

lib/Controller/ApiController.php

@@ -47,9 +47,9 @@ class ApiController extends OCSController {
 	/** @var Defaults */
 	private $defaults;
 
-	const REGISTRATION_STATUS_COMPLETE = 0;
-	const REGISTRATION_STATUS_PENDING = 1;
-	const REGISTRATION_STATUS_EXISTING = 2;
+	public const REGISTRATION_STATUS_COMPLETE = 0;
+	public const REGISTRATION_STATUS_PENDING = 1;
+	public const REGISTRATION_STATUS_EXISTING = 2;
 
 	public function __construct($appName,
 								IRequest $request,

lib/Controller/RegisterController.php

@@ -59,10 +59,10 @@ class RegisterController extends Controller {
 	 * @return TemplateResponse
 	 */
 	public function askEmail($errormsg, $entered) {
-		$params = array(
+		$params = [
 			'errormsg' => $errormsg ? $errormsg : $this->request->getParam('errormsg'),
 			'entered' => $entered ? $entered : $this->request->getParam('entered')
-		);
+		];
 		return new TemplateResponse('registration', 'register', $params, 'guest');
 	}
 
@@ -88,20 +88,20 @@ class RegisterController extends Controller {
 			} else {
 				$this->registrationService->generateNewToken($reg);
 				$this->mailService->sendTokenByMail($reg);
-				return new TemplateResponse('registration', 'message', array('msg' =>
+				return new TemplateResponse('registration', 'message', ['msg' =>
 					$this->l10n->t('There is already a pending registration with this email, a new verification email has been sent to the address.')
-				), 'guest');
+				], 'guest');
 			}
 		} catch (RegistrationException $e) {
-			return new TemplateResponse('registration', 'message', array('msg' =>
+			return new TemplateResponse('registration', 'message', ['msg' =>
 				$e->getMessage().'<br/>'.$e->getHint()
-			), 'guest');
+			], 'guest');
 		}
 
 
-		return new TemplateResponse('registration', 'message', array('msg' =>
+		return new TemplateResponse('registration', 'message', ['msg' =>
 			$this->l10n->t('Verification email successfully sent.')
-		), 'guest');
+		], 'guest');
 	}
 
 	/**
@@ -130,7 +130,6 @@ class RegisterController extends Controller {
 		} catch (RegistrationException $exception) {
 			return $this->renderError($exception->getMessage(), $exception->getHint());
 		}
-
 	}
 
 	/**
@@ -152,8 +151,8 @@ class RegisterController extends Controller {
 			return new TemplateResponse('registration', 'form',
 				[
 					'email' => $registration->getEmail(),
-					'entered_data' => array('user' => $username),
-					'errormsgs' => array($exception->getMessage()),
+					'entered_data' => ['user' => $username],
+					'errormsgs' => [$exception->getMessage()],
 					'token' => $token
 				], 'guest');
 		}
@@ -166,18 +165,17 @@ class RegisterController extends Controller {
 			return new TemplateResponse(
 				'registration',
 				'message',
-				array('msg' => $this->l10n->t("Your account has been successfully created, but it still needs approval from an administrator.")),
+				['msg' => $this->l10n->t("Your account has been successfully created, but it still needs approval from an administrator.")],
 				'guest');
 		}
 	}
 
 	private function renderError($error, $hint="") {
-		return new TemplateResponse('', 'error', array(
-			'errors' => array(array(
+		return new TemplateResponse('', 'error', [
+			'errors' => [[
 				'error' => $error,
 				'hint' => $hint
-			))
-		), 'error');
+			]]
+		], 'error');
 	}
-
 }

lib/Controller/SettingsController.php

@@ -52,7 +52,7 @@ class SettingsController extends Controller {
 	 */
 	public function admin($registered_user_group, $allowed_domains, $admin_approval_required) {
 		// handle domains
-		if ( ( $allowed_domains==='' ) || ( $allowed_domains === NULL ) ){
+		if (($allowed_domains==='') || ($allowed_domains === null)) {
 			$this->config->deleteAppValue($this->appName, 'allowed_domains');
 		} else {
 			$this->config->setAppValue($this->appName, 'allowed_domains', $allowed_domains);
@@ -63,34 +63,34 @@ class SettingsController extends Controller {
 
 		// handle groups
 		$groups = $this->groupmanager->search('');
-		$group_id_list = array();
+		$group_id_list = [];
 		foreach ($groups as $group) {
 			$group_id_list[] = $group->getGid();
 		}
 		if ($registered_user_group === 'none') {
 			$this->config->deleteAppValue($this->appName, 'registered_user_group');
-			return new DataResponse(array(
-				'data' => array(
+			return new DataResponse([
+				'data' => [
 					'message' => (string) $this->l10n->t('Saved'),
-				),
+				],
 				'status' => 'success'
 				
-			));
+			]);
 		} elseif (in_array($registered_user_group, $group_id_list)) {
 			$this->config->setAppValue($this->appName, 'registered_user_group', $registered_user_group);
-			return new DataResponse(array(
-				'data' => array(
+			return new DataResponse([
+				'data' => [
 					'message' => (string) $this->l10n->t('Saved'),
-				),
+				],
 				'status' => 'success'
-			));
+			]);
 		} else {
-			return new DataResponse(array(
-				'data' => array(
+			return new DataResponse([
+				'data' => [
 					'message' => (string) $this->l10n->t('No such group'),
-				),
+				],
 				'status' => 'error'
-			), Http::STATUS_NOT_FOUND);
+			], Http::STATUS_NOT_FOUND);
 		}
 	}
 	/**

lib/Db/Registration.php

@@ -26,7 +26,6 @@ namespace OCA\Registration\Db;
 use OCP\AppFramework\Db\Entity;
 
 class Registration extends Entity {
-
 	public $id;
 	protected $email;
 	protected $username;
@@ -40,5 +39,4 @@ class Registration extends Entity {
 	public function __construct() {
 		$this->addType('emailConfirmed', 'boolean');
 	}
-
 }

lib/Db/RegistrationMapper.php

@@ -97,5 +97,4 @@ class RegistrationMapper extends Mapper {
 		//FIXME eqivalent to ISecureRandom::CHAR_HUMAN_READABLE introduced in https://github.com/nextcloud/server/commit/f2a2b34e4639e88f8d948a388a51f010212b42a3 but not supported in ownCloud yet. We'll just use the string for now then switch to constants when supported.
 		$registration->setClientSecret($token);
 	}
-
 }

lib/Service/MailService.php

@@ -79,7 +79,7 @@ class MailService {
 	 * @throws RegistrationException
 	 */
 	public function sendTokenByMail(Registration $registration) {
-		$link = $this->urlGenerator->linkToRoute('registration.register.verifyToken', array('token' => $registration->getToken()));
+		$link = $this->urlGenerator->linkToRoute('registration.register.verifyToken', ['token' => $registration->getToken()]);
 		$link = $this->urlGenerator->getAbsoluteURL($link);
 		$template_var = [
 			'link' => $link,
@@ -125,7 +125,7 @@ class MailService {
 			}
 		}
 
-		$to_arr = array();
+		$to_arr = [];
 		foreach ($admin_users as $au) {
 			$au_email = $au->getEMailAddress();
 			if ($au_email && $au->isEnabled()) {
@@ -182,8 +182,8 @@ class MailService {
 		$message->setPlainBody($plaintext_part);
 		$message->setHtmlBody($html_part);
 		$failed_recipients = $this->mailer->send($message);
-		if ( !empty($failed_recipients) )
+		if (!empty($failed_recipients)) {
 			throw new RegistrationException('Failed recipients: '.print_r($failed_recipients, true));
 		}
-
+	}
 }

lib/Service/RegistrationException.php

@@ -24,7 +24,6 @@
 namespace OCA\Registration\Service;
 
 class RegistrationException extends \Exception {
-
 	protected $hint;
 
 	/**
@@ -45,5 +44,4 @@ class RegistrationException extends \Exception {
 	public function getHint() {
 		return $this->hint;
 	}
-
 }

lib/Service/RegistrationService.php

@@ -139,8 +139,9 @@ class RegistrationService {
 			$registration->setPassword($password);
 		}
 		$this->registrationMapper->generateNewToken($registration);
-		if ( $password !== '' && $username !== '' )
+		if ($password !== '' && $username !== '') {
 			$this->registrationMapper->generateClientSecret($registration);
+		}
 		$this->registrationMapper->insert($registration);
 		return $registration;
 	}
@@ -151,13 +152,13 @@ class RegistrationService {
 	 * @throws RegistrationException
 	 */
 	public function validateEmail($email) {
-
 		$this->mailService->validateEmail($email);
 
 		// check for pending registrations
 		try {
 			return $this->registrationMapper->find($email);//if not found DB will throw a exception
-		} catch (DoesNotExistException $e) {}
+		} catch (DoesNotExistException $e) {
+		}
 
 		if ($this->userManager->getByEmail($email)) {
 			throw new RegistrationException(
@@ -434,7 +435,6 @@ class RegistrationService {
 			['msg' => $this->l10n->t('Your account has been successfully created, you can <a href="%s">log in now</a>.', [$this->urlGenerator->getAbsoluteURL('/')])]
 			, 'guest'
 		);
-
 	}
 
 	/**
@@ -451,5 +451,4 @@ class RegistrationService {
 			}
 		}
 	}
-
 }

lib/Settings/RegistrationSettings.php

@@ -1,7 +1,7 @@
 <?php
+
 namespace OCA\Registration\Settings;
 
-use OCP\AppFramework\Http\TemplateResponse;
 use OCP\Settings\ISettings;
 
 use OCA\Registration\Controller\SettingsController;
@@ -31,5 +31,3 @@ class RegistrationSettings implements ISettings {
 		return $this->getSection();
 	}
 }
-
-?>

lib/Util/CoreBridge.php

@@ -2,8 +2,7 @@
 
 namespace OCA\Registration\Util;
 
-class CoreBridge
-{
+class CoreBridge {
 	/**
 	 * This function maps an exception class to the available exception class of the core
 	 * in order to provide cross-core and cross-version compatibility.
@@ -12,8 +11,7 @@ class CoreBridge
 	 * @return string
 	 * @throws \LogicException
 	 */
-    public static function exceptionClass($className)
-    {
+	public static function exceptionClass($className) {
 		static $classes = [
 			'OCSException'           => [
 				'OCP\AppFramework\OCS\OCSException',
@@ -52,8 +50,7 @@ class CoreBridge
 	 * @param null|int    $code
 	 * @return \Exception
 	 */
-    public static function createException($className, $message = null, $code = null)
-    {
+	public static function createException($className, $message = null, $code = null) {
 		$exceptionClassName = self::exceptionClass($className);
 
 		$reflection = new \ReflectionClass($exceptionClassName);

templates/admin.php

@@ -25,7 +25,9 @@ foreach ( $_['groups'] as $group ) {
 
 	<p>
 	<label for="admin_approval_required"><?php p($l->t('Require admin approval?')); ?>
-	<input type="checkbox" id="admin_approval_required" name="admin_approval_required" <?php if($_['approval_required'] === "yes" ) echo " checked"; ?>>
+	<input type="checkbox" id="admin_approval_required" name="admin_approval_required" <?php if ($_['approval_required'] === "yes") {
+	echo " checked";
+} ?>>
 	</label>
 	</p>
 </form>

templates/email.newuser.disabled_html.php

@@ -1,3 +1,4 @@
 <?php
+
 echo $l->t('A new user "%s" has created an account on %s and awaits admin approbation', [$_['user'], $_['sitename']]);
 echo str_replace('{link}', $_['link'], '<br/><br/><a href="{link}">{link}</a>');

templates/email.newuser.disabled_plaintext.php

@@ -1,3 +1,4 @@
 <?php
+
 echo $l->t('A new user "%s" has created an account on %s and awaits admin approbation', [$_['user'], $_['sitename']]);
 echo "\n\n".$_['link'];

templates/email.newuser_html.php

@@ -1,2 +1,3 @@
 <?php
+
 echo $l->t('A new user "%s" has created an account on %s', [$_['user'], $_['sitename']]);

templates/email.newuser_plaintext.php

@@ -1,2 +1,3 @@
 <?php
+
 echo $l->t('A new user "%s" has created an account on %s', [$_['user'], $_['sitename']]);

templates/email.validate_html.php

@@ -1,3 +1,4 @@
 <?php
+
 echo $l->t('To create a new account on %s, just click the following link:', [$_['sitename']]);
 echo str_replace('{link}', $_['link'], '<br/><br/><a href="{link}">{link}</a>');

templates/email.validate_plaintext.php

@@ -1,3 +1,4 @@
 <?php
+
 echo $l->t("To create a new account on %s, just click the following link:", [$_['sitename']]);
 echo "\n\n".$_['link'];

templates/form.php

@@ -1,9 +1,10 @@
 <?php
 \OCP\Util::addStyle('registration', 'style');
 \OCP\Util::addScript('registration', 'form');
-if ( \OCP\Util::getVersion()[0] >= 12 )
+if (\OCP\Util::getVersion()[0] >= 12) {
 	\OCP\Util::addStyle('core', 'guest');
-?><form action="<?php print_unescaped(\OC::$server->getURLGenerator()->linkToRoute('registration.register.createAccount', array('token'=>$_['token']))) ?>" method="post">
+}
+?><form action="<?php print_unescaped(\OC::$server->getURLGenerator()->linkToRoute('registration.register.createAccount', ['token'=>$_['token']])) ?>" method="post">
 	<input type="hidden" name="requesttoken" value="<?php p($_['requesttoken']) ?>" />
 	<fieldset>
 		<?php if (!empty($_['errormsgs'])) {?>
@@ -24,7 +25,9 @@ if ( \OCP\Util::getVersion()[0] >= 12 )
 		</p>
 
 		<p class="groupmiddle">
-			<input type="text" name="username" id="username" value="<?php if (!empty($_['entered_data']['user'])) { p($_['entered_data']['user']); } ?>" placeholder="<?php p($l->t('Username')); ?>" />
+			<input type="text" name="username" id="username" value="<?php if (!empty($_['entered_data']['user'])) {
+	p($_['entered_data']['user']);
+} ?>" placeholder="<?php p($l->t('Username')); ?>" />
 			<label for="username" class="infield"><?php p($l->t('Username')); ?></label>
 			<img id="username-icon" class="svg" src="<?php print_unescaped(image_path('', 'actions/user.svg')); ?>" alt=""/>
 		</p>

templates/message.php

@@ -1,7 +1,8 @@
 <?php
 \OCP\Util::addStyle('registration', 'style');
-if ( \OCP\Util::getVersion()[0] >= 12 )
-        \OCP\Util::addStyle('core', 'guest')
+if (\OCP\Util::getVersion()[0] >= 12) {
+	\OCP\Util::addStyle('core', 'guest');
+}
 ?>
 <ul class="msg error-wide nc-theming-main-text">
 	<li><?php print_unescaped($_['msg'])?></li>

templates/register.php

@@ -1,7 +1,8 @@
 <?php
 \OCP\Util::addStyle('registration', 'style');
-if ( \OCP\Util::getVersion()[0] >= 12 )
+if (\OCP\Util::getVersion()[0] >= 12) {
 	\OCP\Util::addStyle('core', 'guest');
+}
 if ($_['entered']): ?>
 	<?php if (empty($_['errormsg'])): ?>
 		<ul class="success">

tests/Integration/Controller/RegisterControllerTest.php

@@ -2,13 +2,9 @@
 
 namespace OCA\Registration\Tests\Integration\Controller;
 
-use OCA\Registration\Db\Registration;
 use OCA\Registration\Db\RegistrationMapper;
 use OCA\Registration\Service\MailService;
 use OCA\Registration\Service\RegistrationService;
-use OCA\Registration\Util\CoreBridge;
-use OCP\AppFramework\Http;
-use OCP\AppFramework\Http\DataResponse;
 use OCP\Defaults;
 use OCP\IConfig;
 use OCP\IGroupManager;
@@ -19,7 +15,6 @@ use OCP\IRequest;
 use OCP\Security\ISecureRandom;
 use OCP\Security\ICrypto;
 use OCP\ISession;
-use OCP\IUser;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\IUserSession;
@@ -28,6 +23,7 @@ use \OCP\AppFramework\Http\TemplateResponse;
 
 use ChristophWurst\Nextcloud\Testing\DatabaseTransaction;
 use ChristophWurst\Nextcloud\Testing\TestCase;
+
 /**
  * class RegistrationControllerTest
  *
@@ -134,9 +130,9 @@ class RegisterControllerTest extends TestCase {
 
 		$ret = $this->controller->validateEmail($email);
 
-		$expected = new TemplateResponse('registration', 'message', array('msg' =>
+		$expected = new TemplateResponse('registration', 'message', ['msg' =>
 			$this->l10n->t('Verification email successfully sent.')
-		), 'guest');
+		], 'guest');
 
 
 		$this->assertEquals($expected, $ret, print_r($ret, true));

tests/Integration/Service/RegistrationServiceTest.php

@@ -6,9 +6,6 @@ use OCA\Registration\Db\Registration;
 use OCA\Registration\Db\RegistrationMapper;
 use OCA\Registration\Service\MailService;
 use OCA\Registration\Service\RegistrationService;
-use OCA\Registration\Util\CoreBridge;
-use OCP\AppFramework\Http;
-use OCP\AppFramework\Http\DataResponse;
 use OCP\Defaults;
 use OCP\IConfig;
 use OCP\IGroupManager;
@@ -26,6 +23,7 @@ use OCP\IUserSession;
 
 use ChristophWurst\Nextcloud\Testing\DatabaseTransaction;
 use ChristophWurst\Nextcloud\Testing\TestCase;
+
 /**
  * class RegistrationServiceTest
  *
@@ -202,7 +200,7 @@ class RegistrationServiceTest extends TestCase {
 
 		$this->config->expects($this->atLeastOnce())
 			->method('getAppValue')
-			->will($this->returnCallback(array($this, 'settingsCallback1')));
+			->will($this->returnCallback([$this, 'settingsCallback1']));
 
 
 		$form_input_username = 'alice1';