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.

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

Already on GitHub? Sign in to your account

Added return types #1439

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/Binary/Loader/FileSystemLoader.php
Expand Up @@ -37,7 +37,7 @@ public function __construct(
/**
* {@inheritdoc}
*/
public function find($path)
public function find($path): FileBinary
{
$path = $this->locator->locate($path);
$mimeType = $this->mimeTypeGuesser->guessMimeType($path);
Expand Down
2 changes: 1 addition & 1 deletion src/Binary/Loader/FlysystemLoader.php
Expand Up @@ -33,7 +33,7 @@ public function __construct(
/**
* {@inheritdoc}
*/
public function find($path)
public function find($path): Binary
{
if (false === $this->filesystem->has($path)) {
throw new NotLoadableException(sprintf('Source image "%s" not found.', $path));
Expand Down
2 changes: 1 addition & 1 deletion src/Binary/Loader/FlysystemV2Loader.php
Expand Up @@ -34,7 +34,7 @@ public function __construct(
/**
* {@inheritdoc}
*/
public function find($path)
public function find($path): Binary
{
try {
$mimeType = $this->filesystem->mimeType($path);
Expand Down
2 changes: 1 addition & 1 deletion src/Binary/Loader/StreamLoader.php
Expand Up @@ -46,7 +46,7 @@ public function __construct(string $wrapperPrefix, $context = null)
/**
* {@inheritdoc}
*/
public function find($path)
public function find($path): string
{
$name = $this->wrapperPrefix.$path;

Expand Down
32 changes: 1 addition & 31 deletions src/Events/CacheResolveEvent.php
Expand Up @@ -15,75 +15,45 @@

class CacheResolveEvent extends Event
{
/**
* Resource path.
*/
protected string $path;

/**
* Filter name.
*/
protected string $filter;

/**
* Resource url.
*/
protected ?string $url;

/**
* Init default event state.
*/
public function __construct(string $path, string $filter, ?string $url = null)
{
$this->path = $path;
$this->filter = $filter;
$this->url = $url;
}

/**
* Sets resource path.
*/
public function setPath(string $path): void
{
$this->path = $path;
}

/**
* Returns resource path.
*/
public function getPath(): string
{
return $this->path;
}

/**
* Sets filter name.
*/
public function setFilter(string $filter): void
{
$this->filter = $filter;
}

/**
* Returns filter name.
*/
public function getFilter(): string
{
return $this->filter;
}

/**
* Sets resource url.
*/
public function setUrl(?string $url): void
{
$this->url = $url;
}

/**
* Returns resource url.
*/
public function getUrl(): ?string
public function getUrl(): string
Copy link
Member

Choose a reason for hiding this comment

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

i don't think you can change this to non-nullable as the url property of the class is nullable.

Copy link
Author

Choose a reason for hiding this comment

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

I've just looked at ‘protected ?string $url;’

Copy link
Member

Choose a reason for hiding this comment

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

the correct return type should look like this:

Suggested change
public function getUrl(): string
public function getUrl(): ?string

{
return $this->url;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Imagine/Cache/Resolver/ProxyResolver.php
Expand Up @@ -56,7 +56,7 @@ public function remove(array $paths, array $filters): void
$this->resolver->remove($paths, $filters);
}

protected function rewriteUrl(string $url): string
protected function rewriteUrl(string $url): ?string
Copy link
Member

Choose a reason for hiding this comment

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

the only way how this can return null is when there is a regex error in the preg_replace call. is the calling side of this handling that? if not, i'd prefer to check the return value of preg_replace in this method and use preg_last_error_msg to throw an exception. then we can leave this as returning a non-nullable string.

{
if (empty($this->hosts)) {
return $url;
Expand Down