Skip to content

Commit a047c66

Browse files
authored
Code clean-up (GH-22)
* Declare methods and instance variables in abstract classes While PHP doesn't require this, it is good style, and makes the code easier to reason about. * Declare class variable While `Config::getSdkNugetFeedUrl()` is not used from within the project, and probably just was some experimental stuff, the method is public, so we better keep it for now, but avoid reading an undeclared class variable. * Dodge from covariance warnings regarding Iterator for now * Don't pass unnecessary arguments These are retrieved from within the called functions, what *might* not be the best idea, but it's what we have for now. * Assert that $cb is actually a string before string conversion In practice, it's either "copy" or "rename". * Declare missing `Server::getPhp()` * Remove unreachable code * Ensure that `$crt` is defined From looking at the code, it seems so, but let's make sure before reading an undefined variable. * Config::getDepsPort() returns an int While that is changing the signature of a public method, it makes no sense to return a string, and later to convert to int again. * The abstract class TrainingCase is an Interfaces\TrainingCase There shouldn't be the need for even having an interface (an abstract class should be sufficient), but we keep the interface for now.
1 parent 20d13ba commit a047c66

File tree

8 files changed

+24
-9
lines changed

8 files changed

+24
-9
lines changed

lib/php/libsdk/SDK/Build/PGO/Abstracts/PHP.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ protected function setupPaths()
3030
}
3131
}
3232

33+
abstract public function getExeFilename() : string;
34+
3335
/* TODO Might be improved. */
3436
public function isDist() : bool
3537
{
@@ -96,7 +98,7 @@ public function getRootDir() : string
9698
public function getVersion(bool $short = false) : string
9799
{
98100
$ret = NULL;
99-
$cli = new CLI($this->conf, $this->scenario);
101+
$cli = new CLI($this->conf);
100102

101103
$out = shell_exec($cli->getExeFilename() . " -n -v");
102104

@@ -119,7 +121,7 @@ public function getVersion(bool $short = false) : string
119121

120122
public function isThreadSafe() : bool
121123
{
122-
$cli = new CLI($this->conf, $this->scenario);
124+
$cli = new CLI($this->conf);
123125

124126
$out = shell_exec($cli->getExeFilename() . " -n -v");
125127

lib/php/libsdk/SDK/Build/PGO/Abstracts/Server.php

+3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
abstract class Server
88
{
9+
protected $name;
10+
protected $php;
11+
912
public function getName() : string
1013
{
1114
return $this->name;

lib/php/libsdk/SDK/Build/PGO/Abstracts/TrainingCase.php

+5-1
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
namespace SDK\Build\PGO\Abstracts;
44

55
use SDK\FileOps;
6+
use SDK\Build\PGO\Interfaces;
67
use SDK\Build\PGO\Tool;
78

8-
abstract class TrainingCase
9+
abstract class TrainingCase implements Interfaces\TrainingCase
910
{
1011
use FileOps;
1112

@@ -15,6 +16,9 @@ abstract class TrainingCase
1516
protected $stat = array();
1617
/** @var \SDK\Build\PGO\Config */
1718
protected $conf;
19+
protected $php;
20+
21+
abstract public function getName() : string;
1822

1923
public function getType() : string
2024
{

lib/php/libsdk/SDK/Build/PGO/Controller.php

+2-4
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ protected function setupConfig($cmd)
5252
switch ($cmd) {
5353
default:
5454
throw new Exception("Unknown action '{$cmd}'.");
55-
break;
5655
case "check_init":
5756
$cnf = new PGOConfig(PGOConfig::MODE_CHECK_INIT);
5857
break;
@@ -80,7 +79,6 @@ public function handle($force)
8079
switch ($this->cmd) {
8180
default:
8281
throw new Exception("Unknown action '{$this->cmd}'.");
83-
break;
8482
case "init":
8583
$lk = new Lock("pgo_init");
8684
if (!$lk->locked()) {
@@ -242,7 +240,7 @@ public function up()
242240
}
243241
echo "\nStarting up PGO environment.\n\n";
244242

245-
foreach ($this->vitalizeSrv("all") as $srv) {
243+
foreach ($this->vitalizeSrv() as $srv) {
246244
$srv->up();
247245
echo "\n";
248246
}
@@ -260,7 +258,7 @@ public function down(bool $force = false)
260258
/* XXX check it was started of course. */
261259
echo "\nShutting down PGO environment.\n\n";
262260

263-
foreach ($this->vitalizeSrv("all") as $srv) {
261+
foreach ($this->vitalizeSrv() as $srv) {
264262
$srv->down($force);
265263
echo "\n";
266264
}

lib/php/libsdk/SDK/Build/PGO/Interfaces/Server.php

+1
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,5 @@ public function init() : void;
1212
public function up() : void;
1313
public function down(bool $force = false) : void;
1414
public function getName() : string;
15+
public function getPhp();
1516
}

lib/php/libsdk/SDK/Build/PGO/TrainingCaseIterator.php

+5
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ protected function getHandlerFilename(string $base) : string
5050
return $base . DIRECTORY_SEPARATOR . "TrainingCaseHandler.php";
5151
}
5252

53+
#[\ReturnTypeWillChange]
5354
public function current()
5455
{
5556
$base = $this->items[$this->idx];
@@ -73,21 +74,25 @@ public function current()
7374
return $this->el;
7475
}
7576

77+
#[\ReturnTypeWillChange]
7678
public function next()
7779
{
7880
$this->idx++;
7981
}
8082

83+
#[\ReturnTypeWillChange]
8184
public function rewind()
8285
{
8386
$this->idx = 0;
8487
}
8588

89+
#[\ReturnTypeWillChange]
8690
public function valid()
8791
{
8892
return $this->idx < count($this->items);
8993
}
9094

95+
#[\ReturnTypeWillChange]
9196
public function key()
9297
{
9398
if (!is_object($this->el)) {

lib/php/libsdk/SDK/Config.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class Config
1212
protected static $depsUriScheme = "https";
1313
protected static $depsBaseUri = "/~windows/php-sdk/deps";
1414

15-
/* protected static $sdkNugetFeedUrl = "http://127.0.0.1/sdk/nuget"; */
15+
protected static $sdkNugetFeedUrl = "http://127.0.0.1/sdk/nuget"; // experimental?
1616
1717
protected static $knownBranches = array ();
1818

@@ -28,7 +28,7 @@ public static function getDepsHost() : string
2828
return self::$depsHost;
2929
}/*}}}*/
3030

31-
public static function getDepsPort() : string
31+
public static function getDepsPort() : int
3232
{/*{{{*/
3333
return self::$depsPort;
3434
}/*}}}*/
@@ -264,6 +264,7 @@ public static function getCurrentBranchData() : array
264264
throw new Exception("Unknown branch '$current_branch_name'");
265265
}
266266

267+
$crt = null;
267268
$cur_crt = Config::getCurrentCrtName();
268269
if (count($branches[$current_branch_name]) > 1) {
269270
if (NULL === $cur_crt) {

lib/php/libsdk/SDK/FileOps.php

+1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ protected function cp_or_mv(string $src, string $dst, callable $cb) : bool
9090

9191
if ($item->isFile()) {
9292
if (!call_user_func($cb, $src_path, $dst_path)) {
93+
assert(is_string($cb));
9394
throw new Exception("Unable to $cb '$src_path' to '$dst_path'");
9495
}
9596
}

0 commit comments

Comments
 (0)