Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: simplify context class #315

Merged
merged 1 commit into from Mar 9, 2024
Merged

chore: simplify context class #315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
View file
Open in desktop
Expand Up @@ -10,11 +10,6 @@ parameters:
count: 1
path: src/Context.php

-
message: "#^Parameter \\#1 \\$data of class Castor\\\\Context constructor expects array\\{name\\: string, production\\: bool, foo\\?\\: string\\}, array\\<int\\|string, mixed\\> given\\.$#"
count: 1
path: src/Context.php

-
message: "#^Function Castor\\\\get_exit_code\\(\\) has parameter \\$args with no type specified\\.$#"
count: 1
Expand Down
173 changes: 28 additions & 145 deletions src/Context.php
View file
Open in desktop
Expand Up @@ -58,198 +58,64 @@ public function withData(array $data, bool $keepExisting = true, bool $recursive

if ($keepExisting) {
if ($recursive) {
/* @var array<(int|string), mixed> */
$data = $this->arrayMergeRecursiveDistinct($this->data, $data);
} else {
/* @var array<(int|string), mixed> */
$data = array_merge($this->data, $data);
}
}

return new self(
$data,
$this->environment,
$this->currentDirectory,
$this->tty,
$this->pty,
$this->timeout,
$this->quiet,
$this->allowFailure,
$this->notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(data: $data);
}

/** @param array<string, string|\Stringable|int> $environment */
public function withEnvironment(array $environment, bool $keepExisting = true): self
{
return new self(
$this->data,
$keepExisting ? [...$this->environment, ...$environment] : $environment,
$this->currentDirectory,
$this->tty,
$this->pty,
$this->timeout,
$this->quiet,
$this->allowFailure,
$this->notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(environment: $keepExisting ? [...$this->environment, ...$environment] : $environment);
}

public function withPath(string $path): self
{
return new self(
$this->data,
$this->environment,
str_starts_with($path, '/') ? $path : PathHelper::realpath($this->currentDirectory . '/' . $path),
$this->tty,
$this->pty,
$this->timeout,
$this->quiet,
$this->allowFailure,
$this->notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(currentDirectory: str_starts_with($path, '/') ? $path : PathHelper::realpath($this->currentDirectory . '/' . $path));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice something I don't like.
withPath but the property is currentDirectory.
What about deprecating withPath to withCurrentDirectory?
(if okay, I'll open a new PR)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for renaming it, it bugged me a few times 馃憤

Copy link
Contributor

@TheoD02 TheoD02 Mar 8, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current name of the run method for defining the path is also path. Shouldn't we also make sure they're identical?

Why not something like workingPath or workingDirectory to indicate the working directory?

}

public function withTty(bool $tty = true): self
{
return new self(
$this->data,
$this->environment,
$this->currentDirectory,
$tty,
$this->pty,
$this->timeout,
$this->quiet,
$this->allowFailure,
$this->notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(tty: $tty);
}

public function withPty(bool $pty = true): self
{
return new self(
$this->data,
$this->environment,
$this->currentDirectory,
$this->tty,
$pty,
$this->timeout,
$this->quiet,
$this->allowFailure,
$this->notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(pty: $pty);
}

public function withTimeout(?float $timeout): self
{
return new self(
$this->data,
$this->environment,
$this->currentDirectory,
$this->tty,
$this->pty,
$timeout,
$this->quiet,
$this->allowFailure,
$this->notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(timeout: $timeout);
}

public function withQuiet(bool $quiet = true): self
{
return new self(
$this->data,
$this->environment,
$this->currentDirectory,
$this->tty,
$this->pty,
$this->timeout,
$quiet,
$this->allowFailure,
$this->notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(quiet: $quiet);
}

public function withAllowFailure(bool $allowFailure = true): self
{
return new self(
$this->data,
$this->environment,
$this->currentDirectory,
$this->tty,
$this->pty,
$this->timeout,
$this->quiet,
$allowFailure,
$this->notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(allowFailure: $allowFailure);
}

public function withNotify(bool $notify = true): self
{
return new self(
$this->data,
$this->environment,
$this->currentDirectory,
$this->tty,
$this->pty,
$this->timeout,
$this->quiet,
$this->allowFailure,
$notify,
$this->verbosityLevel,
$this->name,
);
return $this->with(notify: $notify);
}

public function withVerbosityLevel(VerbosityLevel $verbosityLevel): self
{
return new self(
$this->data,
$this->environment,
$this->currentDirectory,
$this->tty,
$this->pty,
$this->timeout,
$this->quiet,
$this->allowFailure,
$this->notify,
$verbosityLevel,
$this->name,
);
return $this->with(verbosityLevel: $verbosityLevel);
}

public function withName(string $name): self
{
return new self(
$this->data,
$this->environment,
$this->currentDirectory,
$this->tty,
$this->pty,
$this->timeout,
$this->quiet,
$this->allowFailure,
$this->notify,
$this->verbosityLevel,
$name,
);
return $this->with(name: $name);
}

public function offsetExists(mixed $offset): bool
Expand All @@ -276,6 +142,23 @@ public function offsetUnset(mixed $offset): void
throw new \LogicException('Context is immutable.');
}

private function with(mixed ...$args): self
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider putting all the parameters from the constructor here, with their type, but then nullable, with default null value?

It would still allow you to only use named parameters, but now you have type safety.

Instead of an array you reference the parameter directly and fallback to the original value using ??.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but did not succeed to do that. There is not way in PHP to check is null is passed or if it's the default value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it doesn't work when a property is nullable. Forgot about that.

Too bad!

{
return new self(
$args['data'] ?? $this->data,
$args['environment'] ?? $this->environment,
$args['currentDirectory'] ?? $this->currentDirectory,
$args['tty'] ?? $this->tty,
$args['pty'] ?? $this->pty,
$args['timeout'] ?? $this->timeout,
$args['quiet'] ?? $this->quiet,
$args['allowFailure'] ?? $this->allowFailure,
$args['notify'] ?? $this->notify,
$args['verbosityLevel'] ?? $this->verbosityLevel,
$args['name'] ?? $this->name,
);
}

/**
* @param array<(int|string), mixed> $array1
* @param array<(int|string), mixed> $array2
Expand Down
4 changes: 2 additions & 2 deletions tools/phpstan/castor.php
View file
Open in desktop
Expand Up @@ -7,10 +7,10 @@
use function Castor\run;

#[AsTask(description: 'Run PHPStan', aliases: ['phpstan'])]
function phpstan(): int
function phpstan(bool $generateBaseline = false): int
{
return run(
[__DIR__ . '/vendor/bin/phpstan', '--configuration=' . \dirname(__DIR__, 2) . '/phpstan.neon'],
[__DIR__ . '/vendor/bin/phpstan', '--configuration=' . \dirname(__DIR__, 2) . '/phpstan.neon', $generateBaseline ? '-b' : ''],
allowFailure: true,
)->getExitCode();
}