Some app fixes of phpstorm inspections

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

apps/files_sharing/appinfo/routes.php

@@ -26,8 +26,6 @@
  *
  */
 
-use OCP\API;
-
 $application = new \OCA\Files_Sharing\AppInfo\Application();
 $application->registerRoutes($this, [
 	'resources' => [
@@ -127,7 +125,3 @@ $application->registerRoutes($this, [
 /** @var $this \OCP\Route\IRouter */
 $this->create('sharing_external_shareinfo', '/shareinfo')
 	->actionInclude('files_sharing/ajax/shareinfo.php');
-
-// OCS API
-
-//TODO: SET: mail notification, waiting for PR #4689 to be accepted

apps/files_sharing/lib/Activity/Providers/RemoteShares.php

@@ -23,9 +23,7 @@ namespace OCA\Files_Sharing\Activity\Providers;
 
 use OCP\Activity\IEvent;
 use OCP\Activity\IManager;
-use OCP\Activity\IProvider;
 use OCP\Federation\ICloudIdManager;
-use OCP\IL10N;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;

apps/files_sharing/lib/Capabilities.php

@@ -71,7 +71,7 @@ class Capabilities implements ICapability {
 				$public['upload'] = $this->config->getAppValue('core', 'shareapi_allow_public_upload', 'yes') === 'yes';
 				$public['upload_files_drop'] = $public['upload'];
 			}
-			$res["public"] = $public;
+			$res['public'] = $public;
 
 			$res['resharing'] = $this->config->getAppValue('core', 'shareapi_allow_resharing', 'yes') === 'yes';
 

apps/files_sharing/lib/Command/CleanupRemoteStorages.php

@@ -60,11 +60,11 @@ class CleanupRemoteStorages extends Command {
 
 		$remoteStorages = $this->getRemoteStorages();
 
-		$output->writeln(count($remoteStorages) . " remote storage(s) need(s) to be checked");
+		$output->writeln(count($remoteStorages) . ' remote storage(s) need(s) to be checked');
 
 		$remoteShareIds = $this->getRemoteShareIds();
 
-		$output->writeln(count($remoteShareIds) . " remote share(s) exist");
+		$output->writeln(count($remoteShareIds) . ' remote share(s) exist');
 
 		foreach ($remoteShareIds as $id => $remoteShareId) {
 			if (isset($remoteStorages[$remoteShareId])) {
@@ -79,7 +79,7 @@ class CleanupRemoteStorages extends Command {
 		}
 
 		if (empty($remoteStorages)) {
-			$output->writeln("<info>no storages deleted</info>");
+			$output->writeln('<info>no storages deleted</info>');
 		} else {
 			$dryRun = $input->getOption('dry-run');
 			foreach ($remoteStorages as $id => $numericId) {

apps/files_sharing/lib/ExpireSharesJob.php

@@ -44,7 +44,6 @@ class ExpireSharesJob extends TimedJob {
 	 */
 	public function run($argument) {
 		$connection = \OC::$server->getDatabaseConnection();
-		$logger = \OC::$server->getLogger();
 
 		//Current time
 		$now = new \DateTime();

apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php

@@ -37,6 +37,7 @@ use OCA\Files_Sharing\Exceptions\S2SException;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IRequest;
 use OCP\Share\IManager;
+use OCP\Share\Exceptions\ShareNotFound;
 
 /**
  * Checks whether the "sharing check" is enabled
@@ -88,6 +89,7 @@ class SharingCheckMiddleware extends Middleware {
 	 * @param string $methodName
 	 * @throws NotFoundException
 	 * @throws S2SException
+	 * @throws ShareNotFound
 	 */
 	public function beforeController($controller, $methodName) {
 		if(!$this->isSharingEnabled()) {

apps/files_sharing/lib/ShareBackend/Folder.php

@@ -50,8 +50,8 @@ class Folder extends File implements \OCP\Share_Backend_Collection {
 					$share['file_path'] = $name;
 					$displayNameOwner = \OCP\User::getDisplayName($share['uid_owner']);
 					$displayNameShareWith = \OCP\User::getDisplayName($share['share_with']);
-					$share['displayname_owner'] = ($displayNameOwner) ? $displayNameOwner : $share['uid_owner'];
-					$share['share_with_displayname'] = ($displayNameShareWith) ? $displayNameShareWith : $share['uid_owner'];
+					$share['displayname_owner'] = $displayNameOwner ? $displayNameOwner : $share['uid_owner'];
+					$share['share_with_displayname'] = $displayNameShareWith ? $displayNameShareWith : $share['uid_owner'];
 
 					$result[] = $share;
 				}
@@ -72,7 +72,7 @@ class Folder extends File implements \OCP\Share_Backend_Collection {
 		$query = \OCP\DB::prepare('SELECT `parent` FROM `*PREFIX*filecache` WHERE `fileid` = ?');
 		$result = $query->execute(array($child));
 		$row = $result->fetchRow();
-		$parent = ($row) ? $row['parent'] : null;
+		$parent = $row ? $row['parent'] : null;
 
 		return $parent;
 	}

apps/files_sharing/tests/BackendTest.php

@@ -68,7 +68,7 @@ class BackendTest extends TestCase {
 		parent::tearDown();
 	}
 
-	function testGetParents() {
+	public function testGetParents() {
 
 		$fileinfo1 = $this->view->getFileInfo($this->folder);
 		$fileinfo2 = $this->view->getFileInfo($this->folder . $this->subfolder . $this->subsubfolder);

apps/files_sharing/tests/CapabilitiesTest.php

@@ -24,8 +24,7 @@
 namespace OCA\Files_Sharing\Tests;
 
 use OCA\Files_Sharing\Capabilities;
-use OCA\Files_Sharing\Tests\TestCase;
-use OCP\App\IAppManager;
+
 
 /**
  * Class CapabilitiesTest

apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php

@@ -22,7 +22,6 @@
 namespace OCA\Files_Sharing\Tests\Command;
 
 use OCA\Files_Sharing\Command\CleanupRemoteStorages;
-use OCP\DB\QueryBuilder\IQueryBuilder;
 use Test\TestCase;
 
 /**

apps/files_sharing/tests/HelperTest.php

@@ -35,7 +35,7 @@ class HelperTest extends TestCase {
 	/**
 	 * test set and get share folder
 	 */
-	function testSetGetShareFolder() {
+	public function testSetGetShareFolder() {
 		$this->assertSame('/', \OCA\Files_Sharing\Helper::getShareFolder());
 
 		\OCA\Files_Sharing\Helper::setShareFolder('/Shared/Folder');

apps/files_sharing/tests/MountProviderTest.php

@@ -33,7 +33,6 @@ use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Share\IShare;
 use OCP\Share\IManager;
-use OCP\Files\Mount\IMountPoint;
 
 /**
  * @group DB

apps/files_sharing/tests/SharedMountTest.php

@@ -54,8 +54,8 @@ class SharedMountTest extends TestCase {
 		$this->view->mkdir($this->folder);
 
 		// save file with content
-		$this->view->file_put_contents($this->filename, "root file");
-		$this->view->file_put_contents($this->folder . $this->filename, "file in subfolder");
+		$this->view->file_put_contents($this->filename, 'root file');
+		$this->view->file_put_contents($this->folder . $this->filename, 'file in subfolder');
 
 		$this->groupManager = \OC::$server->getGroupManager();
 		$this->userManager = \OC::$server->getUserManager();
@@ -204,18 +204,18 @@ class SharedMountTest extends TestCase {
 
 		$this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename));
 
-		\OC\Files\Filesystem::rename($this->filename, "newFileName");
+		\OC\Files\Filesystem::rename($this->filename, 'newFileName');
 
 		$this->assertTrue(\OC\Files\Filesystem::file_exists('newFileName'));
 		$this->assertFalse(\OC\Files\Filesystem::file_exists($this->filename));
 
 		self::loginHelper(self::TEST_FILES_SHARING_API_USER3);
 		$this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename));
-		$this->assertFalse(\OC\Files\Filesystem::file_exists("newFileName"));
+		$this->assertFalse(\OC\Files\Filesystem::file_exists('newFileName'));
 
 		self::loginHelper(self::TEST_FILES_SHARING_API_USER3);
 		$this->assertTrue(\OC\Files\Filesystem::file_exists($this->filename));
-		$this->assertFalse(\OC\Files\Filesystem::file_exists("newFileName"));
+		$this->assertFalse(\OC\Files\Filesystem::file_exists('newFileName'));
 
 		//cleanup
 		self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
@@ -240,7 +240,7 @@ class SharedMountTest extends TestCase {
 			if ($exception) {
 				$this->assertSame(10, $e->getCode());
 			} else {
-				$this->assertTrue(false, "Exception catched, but expected: " . $expectedResult);
+				$this->assertTrue(false, 'Exception catched, but expected: ' . $expectedResult);
 			}
 		}
 	}
@@ -323,7 +323,7 @@ class SharedMountTest extends TestCase {
 	 *
 	 * @dataProvider dataPermissionMovedGroupShare
 	 */
-	function testPermissionMovedGroupShare($type, $beforePerm, $afterPerm) {
+	public function testPermissionMovedGroupShare($type, $beforePerm, $afterPerm) {
 
 		if ($type === 'file') {
 			$path = $this->filename;
@@ -355,7 +355,7 @@ class SharedMountTest extends TestCase {
 		$this->assertEquals($beforePerm, $result->getPermissions());
 
 		// Now move the item forcing a new entry in the share table
-		\OC\Files\Filesystem::rename($path, "newPath");
+		\OC\Files\Filesystem::rename($path, 'newPath');
 		$this->assertTrue(\OC\Files\Filesystem::file_exists('newPath'));
 		$this->assertFalse(\OC\Files\Filesystem::file_exists($path));
 
@@ -388,7 +388,7 @@ class SharedMountTest extends TestCase {
 	 * If the permissions on a group share are upgraded be sure to still respect 
 	 * removed shares by a member of that group
 	 */
-	function testPermissionUpgradeOnUserDeletedGroupShare() {
+	public function testPermissionUpgradeOnUserDeletedGroupShare() {
 		$testGroup = $this->groupManager->createGroup('testGroup');
 		$user1 = $this->userManager->get(self::TEST_FILES_SHARING_API_USER1);
 		$user2 = $this->userManager->get(self::TEST_FILES_SHARING_API_USER2);

apps/files_sharing/tests/SizePropagationTest.php

@@ -26,7 +26,6 @@
 namespace OCA\Files_Sharing\Tests;
 
 use OC\Files\View;
-use Test\Traits\MountProviderTrait;
 use Test\Traits\UserTrait;
 
 /**

apps/files_sharing/tests/UnshareChildrenTest.php

@@ -74,7 +74,7 @@ class UnshareChildrenTest extends TestCase {
 	/**
 	 * @medium
 	 */
-	function testUnshareChildren() {
+	public function testUnshareChildren() {
 
 		$fileInfo2 = \OC\Files\Filesystem::getFileInfo($this->folder);
 

apps/files_sharing/tests/UpdaterTest.php

@@ -69,7 +69,7 @@ class UpdaterTest extends TestCase {
 	 * points should be unshared before the folder gets deleted so
 	 * that the mount point doesn't end up at the trash bin
 	 */
-	function testDeleteParentFolder() {
+	public function testDeleteParentFolder() {
 		$status = \OC_App::isEnabled('files_trashbin');
 		(new \OC_App())->enable('files_trashbin');
 
@@ -96,8 +96,8 @@ class UpdaterTest extends TestCase {
 		$foldersShared = \OCP\Share::getItemsSharedWith('folder');
 		$this->assertSame(1, count($foldersShared));
 
-		$view->mkdir("localFolder");
-		$view->file_put_contents("localFolder/localFile.txt", "local file");
+		$view->mkdir('localFolder');
+		$view->file_put_contents('localFolder/localFile.txt', 'local file');
 
 		$view->rename($this->folder, 'localFolder/' . $this->folder);
 
@@ -196,7 +196,7 @@ class UpdaterTest extends TestCase {
 	/**
 	 * if a folder gets renamed all children mount points should be renamed too
 	 */
-	function testRename() {
+	public function testRename() {
 
 		$fileinfo = \OC\Files\Filesystem::getFileInfo($this->folder);
 

apps/provisioning_api/lib/Controller/AppsController.php

@@ -91,9 +91,9 @@ class AppsController extends OCSController {
 		$info = \OCP\App::getAppInfo($app);
 		if(!is_null($info)) {
 			return new DataResponse(OC_App::getAppInfo($app));
-		} else {
-			throw new OCSException('The request app was not found', \OCP\API::RESPOND_NOT_FOUND);
 		}
+
+		throw new OCSException('The request app was not found', \OCP\API::RESPOND_NOT_FOUND);
 	}
 
 	/**

apps/provisioning_api/lib/Controller/GroupsController.php

@@ -30,6 +30,7 @@ use OCP\AppFramework\OCS\OCSException;
 use OCP\AppFramework\OCSController;
 use OCP\IGroup;
 use OCP\IGroupManager;
+use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IUserSession;
 use OCP\IUser;
@@ -43,21 +44,27 @@ class GroupsController extends OCSController {
 	/** @var IUserSession */
 	private $userSession;
 
+	/** @var ILogger */
+	private $logger;
+
 	/**
 	 * @param string $appName
 	 * @param IRequest $request
 	 * @param IGroupManager $groupManager
 	 * @param IUserSession $userSession
+	 * @param ILogger $logger
 	 */
 	public function __construct(
 			$appName,
 			IRequest $request,
 			IGroupManager $groupManager,
-			IUserSession $userSession) {
+			IUserSession $userSession,
+			ILogger $logger) {
 		parent::__construct($appName, $request);
 
 		$this->groupManager = $groupManager;
 		$this->userSession = $userSession;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -120,9 +127,9 @@ class GroupsController extends OCSController {
 			}, $users);
 			$users = array_values($users);
 			return new DataResponse(['users' => $users]);
-		} else {
-			throw new OCSException('User does not have access to specified group', \OCP\API::RESPOND_UNAUTHORISED);
 		}
+
+		throw new OCSException('User does not have access to specified group', \OCP\API::RESPOND_UNAUTHORISED);
 	}
 
 	/**
@@ -136,8 +143,8 @@ class GroupsController extends OCSController {
 	 */
 	public function addGroup($groupid) {
 		// Validate name
-		if(empty($groupid)){
-			\OCP\Util::writeLog('provisioning_api', 'Group name not supplied', \OCP\Util::ERROR);
+		if(empty($groupid)) {
+			$this->logger->error('Group name not supplied', ['app' => 'provisioning_api']);
 			throw new OCSException('Invalid group name', 101);
 		}
 		// Check if it exists
@@ -179,6 +186,7 @@ class GroupsController extends OCSController {
 			throw new OCSException('Group does not exist', 101);
 		}
 
+		/** @var IUser[] $subadmins */
 		$subadmins = $this->groupManager->getSubAdmin()->getGroupsSubAdmins($targetGroup);
 		// New class returns IUser[] so convert back
 		$uids = [];

apps/provisioning_api/tests/Controller/GroupsControllerTest.php

@@ -37,6 +37,7 @@ class GroupsControllerTest extends \Test\TestCase {
 	protected $userSession;
 	/** @var \OC\SubAdmin|\PHPUnit_Framework_MockObject_MockObject */
 	protected $subAdminManager;
+
 	/** @var GroupsController */
 	protected $api;
 
@@ -60,11 +61,15 @@ class GroupsControllerTest extends \Test\TestCase {
 		$request = $this->getMockBuilder('OCP\IRequest')
 			->disableOriginalConstructor()
 			->getMock();
+
+		$logger = $this->createMock(ILogger::class);
+
 		$this->api = new GroupsController(
 			'provisioning_api',
 			$request,
 			$this->groupManager,
-			$this->userSession
+			$this->userSession,
+			$logger
 		);
 	}
 

apps/provisioning_api/tests/Controller/UsersControllerTest.php

@@ -1128,7 +1128,7 @@ class UsersControllerTest extends TestCase {
 	}
 
 	public function testEditUserAdminUserSelfEditChangeValidQuota() {
-		$loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();;
+		$loggedInUser = $this->getMockBuilder('\OCP\IUser')->disableOriginalConstructor()->getMock();
 		$loggedInUser
 			->expects($this->any())
 			->method('getUID')

settings/ajax/togglesubadmins.php

@@ -39,8 +39,8 @@ $targetUserObject = \OC::$server->getUserManager()->get($username);
 $targetGroupObject = \OC::$server->getGroupManager()->get($group);
 
 $isSubAdminOfGroup = false;
-if($targetUserObject !== null && $targetUserObject !== null) {
-	$isSubAdminOfGroup = $subAdminManager->isSubAdminofGroup($targetUserObject, $targetGroupObject);
+if($targetUserObject !== null && $targetGroupObject !== null) {
+	$isSubAdminOfGroup = $subAdminManager->isSubAdminOfGroup($targetUserObject, $targetGroupObject);
 }
 
 // Toggle group

settings/ajax/uninstallapp.php

@@ -47,5 +47,5 @@ if($result !== false) {
 	OC_JSON::success(array('data' => array('appid' => $appId)));
 } else {
 	$l = \OC::$server->getL10N('settings');
-	OC_JSON::error(array("data" => array( "message" => $l->t("Couldn't remove app.") )));
+	OC_JSON::error(array('data' => array( 'message' => $l->t("Couldn't remove app.") )));
 }

settings/ajax/updateapp.php

@@ -51,7 +51,7 @@ try {
 	$config->setSystemValue('maintenance', false);
 } catch(Exception $ex) {
 	$config->setSystemValue('maintenance', false);
-	OC_JSON::error(array("data" => array( "message" => $ex->getMessage() )));
+	OC_JSON::error(array('data' => array( 'message' => $ex->getMessage() )));
 	return;
 }
 
@@ -59,5 +59,5 @@ if($result !== false) {
 	OC_JSON::success(array('data' => array('appid' => $appId)));
 } else {
 	$l = \OC::$server->getL10N('settings');
-	OC_JSON::error(array("data" => array( "message" => $l->t("Couldn't update app.") )));
+	OC_JSON::error(array('data' => array( 'message' => $l->t("Couldn't update app.") )));
 }