Code quality/no spaces inside parenthesis #31273

phil-davis · 2018-04-26T02:55:15Z

Description

Add php_cs_fixer rule no_spaces_inside_parenthesis

Motivation and Context

combine_consecutive_issets in #31271 leaves behind white space.

How Has This Been Tested?

Run php_cs_fixer locally

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

phil-davis · 2018-04-26T02:57:24Z

I put this on top of #31271 as they "sort of go together" but "not really".

IMO this seems like "a good thing" to tidy up some other random spaces around the code base.

phil-davis · 2018-04-26T05:40:43Z

Why do I get no codecov reported?

DeepDiver1975 · 2018-04-26T06:47:07Z

apps/dav/tests/unit/Connector/Sabre/FileTest.php

-		unset($_SERVER['CONTENT_LENGTH']);
-		unset($_SERVER['REQUEST_METHOD']);
+		unset($_SERVER['HTTP_OC_CHUNKED'], $_SERVER['CONTENT_LENGTH'], $_SERVER['REQUEST_METHOD']);
+


still stupid blank lines ....

PHP-CS-Fixer/PHP-CS-Fixer#2804

no_extra_consecutive_blank_lines looks a good thing to add...

phil-davis · 2018-04-26T07:44:05Z

Now I have 589 files changed. Can we get it over 1000?

codecov · 2018-04-26T08:36:28Z

Codecov Report

Merging #31273 into master will increase coverage by <.01%.
The diff coverage is 58.69%.

@@             Coverage Diff              @@
##             master   #31273      +/-   ##
============================================
+ Coverage     62.58%   62.59%   +<.01%     
+ Complexity    18384    18227     -157     
============================================
  Files          1145     1145              
  Lines         68411    68414       +3     
  Branches       1234     1234              
============================================
+ Hits          42818    42821       +3     
  Misses        25232    25232              
  Partials        361      361

Flag	Coverage Δ	Complexity Δ
#javascript	`52.05% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`63.79% <58.69%> (ø)`	`18227 <39> (-157)`	⬇️

Impacted Files	Coverage Δ	Complexity Δ
apps/files_sharing/templates/list.php	`0% <ø> (ø)`	`0 <0> (ø)`	⬇️
lib/private/Files/Storage/Wrapper/Encryption.php	`64.86% <ø> (ø)`	`156 <0> (ø)`	⬇️
lib/private/AppFramework/Http/Request.php	`96.72% <ø> (ø)`	`140 <0> (ø)`	⬇️
apps/federation/lib/TrustedServers.php	`98.41% <ø> (ø)`	`22 <0> (ø)`	⬇️
...amework/Middleware/Security/SecurityMiddleware.php	`98.36% <ø> (ø)`	`22 <0> (ø)`	⬇️
apps/systemtags/templates/list.php	`0% <ø> (ø)`	`0 <0> (ø)`	⬇️
lib/private/Migration/SimpleOutput.php	`42.85% <ø> (ø)`	`3 <0> (ø)`	⬇️
core/templates/update.admin.php	`0% <ø> (ø)`	`0 <0> (ø)`	⬇️
apps/files/templates/simplelist.php	`0% <ø> (ø)`	`0 <0> (ø)`	⬇️
apps/dav/lib/Meta/RootCollection.php	`30.76% <ø> (ø)`	`6 <0> (ø)`	⬇️
... and 265 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c013e07...6d992b5. Read the comment docs.

phil-davis · 2018-04-26T08:46:10Z

I lost code coverage by deleting blank lines???
Poor codecov - it does not really know what to do with this sort of rubbish change.

phil-davis · 2018-04-26T08:51:07Z

core/ajax/share.php

-				&& isset($_GET['checkReshare'])
-				&& isset($_GET['checkShares'])) {
+			if (isset($_GET['itemType'], $_GET['itemSource'], $_GET['checkReshare'], $_GET['checkShares'])
+


Almost got there - I will fix this manually!

phil-davis · 2018-04-26T08:54:33Z

lib/private/Files/Cache/Scanner.php

@@ -189,7 +189,7 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData =
 						$fileId = $cacheData['fileid'];
 						$data['fileid'] = $fileId;
 						// only reuse data if the file hasn't explicitly changed
-						if (isset($data['storage_mtime']) && isset($cacheData['storage_mtime']) && $data['storage_mtime'] === $cacheData['storage_mtime']) {
+						if (isset($data['storage_mtime'], $cacheData['storage_mtime'])   && $data['storage_mtime'] === $cacheData['storage_mtime']) {


and here it puts in an extra space char - to fix...

phil-davis · 2018-04-26T08:55:33Z

lib/private/Files/Cache/Updater.php

@@ -123,7 +123,7 @@ public function update($path, $time = null) {

 		$data = $this->scanner->scan($path, Scanner::SCAN_SHALLOW, -1, false);
 		if (
-			isset($data['oldSize']) && isset($data['size']) &&
+			isset($data['oldSize'], $data['size'])   &&


too much space here

phil-davis · 2018-04-26T08:57:15Z

lib/private/NaturalSort.php

@@ -110,7 +110,7 @@ public function compare($a, $b) {
 		$aa = self::naturalSortChunkify($a);
 		$bb = self::naturalSortChunkify($b);

-		for ($x = 0; isset($aa[$x]) && isset($bb[$x]); $x++) {
+		for ($x = 0; isset($aa[$x], $bb[$x])  ; $x++) {


B\onus space here

phil-davis · 2018-04-26T09:49:29Z

@DeepDiver1975 this is ready for review and merge, if you want to take the plunge...

patrickjahns · 2018-04-30T05:52:30Z

Can we reset codecov results ? If not, can we put it into a clean PR so we know what codecov finds issues with

phil-davis · 2018-04-30T06:35:40Z

PR #31306 has a clean cherry-pick of all these commits. Let's see what CI thinks about it.

phil-davis · 2018-04-30T09:06:46Z

The other PR also gets 58.51% of diff hit (target 62.58%). I guess that of the lines touched by this code format change, on average they are only inside test-covered code 58.51% of the time, a bit less than the repo average of 62.58%.

I don't think we require to do massive unit test writing in order to implement code-style standards???

DeepDiver1975 · 2018-04-30T09:21:27Z

taking care about rebase

phil-davis · 2018-04-30T09:25:26Z

Are we going to backport these code-style changes?

I would like that to happen, to minimize future rubbish conflicts when backporting other PRs.

DeepDiver1975 · 2018-04-30T09:28:43Z

Are we going to backport these code-style changes?

yes - we will - but I'd first like to get the style sorted out on master and let devs adopt and see if this all works out

phil-davis · 2018-04-30T09:41:04Z

sounds good. "backporting" in the end will be adding the "final" .php_cs and running it to fix everything. (i.e. a 1-by-1 backport of each PR in master will not be particularly easy and will not necessarily get things passing in stable10 anyway)

phil-davis · 2018-04-30T10:47:49Z

Someone override codecov and merge if you like it.

phil-davis · 2018-05-16T09:18:28Z

Effective backport is included in #31453

lock · 2019-07-31T00:01:56Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

phil-davis added the 3 - To Review label Apr 26, 2018

phil-davis added this to the development milestone Apr 26, 2018

phil-davis self-assigned this Apr 26, 2018

phil-davis requested a review from DeepDiver1975 April 26, 2018 02:55

phil-davis mentioned this pull request Apr 26, 2018

Code quality/combine consecutive issets combine consecutive unsets #31271

Merged

9 tasks

DeepDiver1975 reviewed Apr 26, 2018

View reviewed changes

phil-davis force-pushed the code-quality/no_spaces_inside_parenthesis branch from de3b457 to 138aa50 Compare April 26, 2018 07:43

phil-davis commented Apr 26, 2018

View reviewed changes

phil-davis force-pushed the code-quality/no_spaces_inside_parenthesis branch from aeac95b to 420a868 Compare April 28, 2018 06:28

phil-davis mentioned this pull request Apr 30, 2018

Isset spaces blankline code quality #31306

Closed

phil-davis added 5 commits April 30, 2018 11:22

php-cs-fixer: add no_spaces_inside_parenthesis

1afcd1c

Applying new php-cs-fixer rule: no_spaces_inside_parenthesis

51c98c3

php-cs-fixer: add no_extra_consecutive_blank_lines

4070c8d

Applying new php-cs-fixer rule: no_extra_consecutive_blank_lines

1c409d5

Remove some excess white space

6d992b5

DeepDiver1975 force-pushed the code-quality/no_spaces_inside_parenthesis branch from 5a369c0 to 6d992b5 Compare April 30, 2018 09:22

DeepDiver1975 approved these changes Apr 30, 2018

View reviewed changes

DeepDiver1975 merged commit ed0c5df into master Apr 30, 2018

DeepDiver1975 deleted the code-quality/no_spaces_inside_parenthesis branch April 30, 2018 10:50

phil-davis mentioned this pull request May 16, 2018

[stable10] owncloud coding standard #31453

Closed

This was referenced Jun 6, 2018

[stable10] owncloud coding standard #31657

Closed

[stable10] owncloud coding standard #31722

Closed

PVince81 modified the milestones: development, QA Jun 13, 2018

phil-davis mentioned this pull request Jun 19, 2018

[stable10] owncloud coding standard #31828

Closed

This was referenced Jul 20, 2018

[stable10] owncloud coding standard #32103

Closed

[stable10] owncloud coding standard #32116

Merged

lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code quality/no spaces inside parenthesis #31273

Code quality/no spaces inside parenthesis #31273

phil-davis commented Apr 26, 2018

phil-davis commented Apr 26, 2018

phil-davis commented Apr 26, 2018

DeepDiver1975 Apr 26, 2018

phil-davis Apr 26, 2018

phil-davis commented Apr 26, 2018

codecov bot commented Apr 26, 2018 •

edited

Loading

phil-davis commented Apr 26, 2018

phil-davis Apr 26, 2018

phil-davis Apr 26, 2018

phil-davis Apr 26, 2018

phil-davis Apr 26, 2018

phil-davis commented Apr 26, 2018

patrickjahns commented Apr 30, 2018

phil-davis commented Apr 30, 2018

phil-davis commented Apr 30, 2018

DeepDiver1975 commented Apr 30, 2018

phil-davis commented Apr 30, 2018

DeepDiver1975 commented Apr 30, 2018

phil-davis commented Apr 30, 2018

phil-davis commented Apr 30, 2018

phil-davis commented May 16, 2018

lock bot commented Jul 31, 2019

Code quality/no spaces inside parenthesis #31273

Code quality/no spaces inside parenthesis #31273

Conversation

phil-davis commented Apr 26, 2018

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

phil-davis commented Apr 26, 2018

phil-davis commented Apr 26, 2018

DeepDiver1975 Apr 26, 2018

Choose a reason for hiding this comment

phil-davis Apr 26, 2018

Choose a reason for hiding this comment

phil-davis commented Apr 26, 2018

codecov bot commented Apr 26, 2018 • edited Loading

Codecov Report

phil-davis commented Apr 26, 2018

phil-davis Apr 26, 2018

Choose a reason for hiding this comment

phil-davis Apr 26, 2018

Choose a reason for hiding this comment

phil-davis Apr 26, 2018

Choose a reason for hiding this comment

phil-davis Apr 26, 2018

Choose a reason for hiding this comment

phil-davis commented Apr 26, 2018

patrickjahns commented Apr 30, 2018

phil-davis commented Apr 30, 2018

phil-davis commented Apr 30, 2018

DeepDiver1975 commented Apr 30, 2018

phil-davis commented Apr 30, 2018

DeepDiver1975 commented Apr 30, 2018

phil-davis commented Apr 30, 2018

phil-davis commented Apr 30, 2018

phil-davis commented May 16, 2018

lock bot commented Jul 31, 2019

codecov bot commented Apr 26, 2018 •

edited

Loading