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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor DependencyInterface #1368

Closed
wants to merge 2 commits into from
Closed

Refactor DependencyInterface #1368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bfb213d
Refactor DependencyInterface to use DependencyContext instead of File…
patrickkusebauch Feb 7, 2024
c0a5a73
Refactor DependencyInterface to use DependencyContext instead of File…
patrickkusebauch Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/Contract/Ast/DependencyContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Qossmic\Deptrac\Contract\Ast;

/**
* @psalm-immutable
*
* Context of the dependency.
*
* Any additional info about where the dependency occurred.
*/
final class DependencyContext
{
public function __construct(
public readonly FileOccurrence $fileOccurrence,
public readonly DependencyType $dependencyType
) {}
}
7 changes: 2 additions & 5 deletions src/Contract/Dependency/DependencyInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@

namespace Qossmic\Deptrac\Contract\Dependency;

use Qossmic\Deptrac\Contract\Ast\DependencyType;
use Qossmic\Deptrac\Contract\Ast\FileOccurrence;
use Qossmic\Deptrac\Contract\Ast\DependencyContext;
use Qossmic\Deptrac\Contract\Ast\TokenInterface;

/**
Expand All @@ -17,12 +16,10 @@ public function getDepender(): TokenInterface;

public function getDependent(): TokenInterface;

public function getFileOccurrence(): FileOccurrence;
public function getContext(): DependencyContext;

/**
* @return array<array{name:string, line:int}>
*/
public function serialize(): array;

public function getType(): DependencyType;
}
6 changes: 2 additions & 4 deletions src/Core/Ast/AstMap/DependencyToken.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@

namespace Qossmic\Deptrac\Core\Ast\AstMap;

use Qossmic\Deptrac\Contract\Ast\DependencyType;
use Qossmic\Deptrac\Contract\Ast\FileOccurrence;
use Qossmic\Deptrac\Contract\Ast\DependencyContext;
use Qossmic\Deptrac\Contract\Ast\TokenInterface;

/**
Expand All @@ -15,7 +14,6 @@ class DependencyToken
{
public function __construct(
public readonly TokenInterface $token,
public readonly FileOccurrence $fileOccurrence,
public readonly DependencyType $type
public readonly DependencyContext $context,
) {}
}
4 changes: 1 addition & 3 deletions src/Core/Ast/AstMap/File/FileReferenceBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace Qossmic\Deptrac\Core\Ast\AstMap\File;

use Qossmic\Deptrac\Contract\Ast\DependencyType;
use Qossmic\Deptrac\Contract\Ast\FileOccurrence;
use Qossmic\Deptrac\Core\Ast\AstMap\ClassLike\ClassLikeReferenceBuilder;
use Qossmic\Deptrac\Core\Ast\AstMap\ClassLike\ClassLikeToken;
use Qossmic\Deptrac\Core\Ast\AstMap\DependencyToken;
Expand All @@ -29,8 +28,7 @@ public function useStatement(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::USE
$this->createContext($occursAtLine, DependencyType::USE),
);

return $this;
Expand Down
54 changes: 22 additions & 32 deletions src/Core/Ast/AstMap/ReferenceBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Qossmic\Deptrac\Core\Ast\AstMap;

use Qossmic\Deptrac\Contract\Ast\DependencyContext;
use Qossmic\Deptrac\Contract\Ast\DependencyType;
use Qossmic\Deptrac\Contract\Ast\FileOccurrence;
use Qossmic\Deptrac\Core\Ast\AstMap\ClassLike\ClassLikeToken;
Expand All @@ -28,6 +29,11 @@ final public function getTokenTemplates(): array
return $this->tokenTemplates;
}

protected function createContext(int $occursAtLine, DependencyType $type): DependencyContext
{
return new DependencyContext(new FileOccurrence($this->filepath, $occursAtLine), $type);
}

/**
* Unqualified function and constant names inside a namespace cannot be
* statically resolved. Inside a namespace Foo, a call to strlen() may
Expand All @@ -39,8 +45,7 @@ public function unresolvedFunctionCall(string $functionName, int $occursAtLine):
{
$this->dependencies[] = new DependencyToken(
FunctionToken::fromFQCN($functionName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::UNRESOLVED_FUNCTION_CALL
$this->createContext($occursAtLine, DependencyType::UNRESOLVED_FUNCTION_CALL),
);

return $this;
Expand All @@ -50,8 +55,7 @@ public function variable(string $classLikeName, int $occursAtLine): self
{
Copy link

Choose a reason for hiding this comment

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

It's not clear to me how a caller could provide additional information about the dependency this way. It seems like we'd have to replace the $occursAtLine parameter with $context in all methods here, so the caller can control the information in the context. That way, we can add more information to the context without having to change all the methods in ReferenceBuilder again.

However, that means that the dependency type should probably not be part of the context.

Copy link

Choose a reason for hiding this comment

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

In any case, thank you for doing this, having DependencyContext available will make it a lot easier to implement the features we discussed.

Copy link

Choose a reason for hiding this comment

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

Hm, I guess we could also implement the "deprecated context" by creating a new ReferenceBuilder when entering a deprecated scope. But I'd still prefer to replace $occursAtLine - the line currrent is the context, we are just providing a wrapper that makes it future-proof.

Copy link
Collaborator Author

@patrickkusebauch patrickkusebauch Mar 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is contextual information, not information that you find out when the dependency is created. So I would imagine something like this:

  • When you enterNode a deprecated node in FileReferenceVisitor, you call $this->currentReference->setIsInDeprecetedContext(true) or something similar. This stores an instance property inside a ReferenceBuilder.
  • When you leaveNode, you set it to false in the same manner.
  • Update ReferenceBuilder::createContext to pass the instance property to the DependencyContext value object constructor
  • Propagate it forward where appropriate
  • Use the information in the event processing to stop raising violations.

Does that make sense?

$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::VARIABLE
$this->createContext($occursAtLine, DependencyType::VARIABLE),
);

return $this;
Expand All @@ -61,17 +65,15 @@ public function superglobal(string $superglobalName, int $occursAtLine): void
{
$this->dependencies[] = new DependencyToken(
SuperGlobalToken::from($superglobalName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::SUPERGLOBAL_VARIABLE
$this->createContext($occursAtLine, DependencyType::SUPERGLOBAL_VARIABLE),
);
}

public function returnType(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::RETURN_TYPE
$this->createContext($occursAtLine, DependencyType::RETURN_TYPE),
);

return $this;
Expand All @@ -81,8 +83,7 @@ public function throwStatement(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::THROW
$this->createContext($occursAtLine, DependencyType::THROW),
);

return $this;
Expand All @@ -92,44 +93,39 @@ public function anonymousClassExtends(string $classLikeName, int $occursAtLine):
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::ANONYMOUS_CLASS_EXTENDS
$this->createContext($occursAtLine, DependencyType::ANONYMOUS_CLASS_EXTENDS),
);
}

public function anonymousClassTrait(string $classLikeName, int $occursAtLine): void
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::ANONYMOUS_CLASS_TRAIT
$this->createContext($occursAtLine, DependencyType::ANONYMOUS_CLASS_TRAIT),
);
}

public function constFetch(string $classLikeName, int $occursAtLine): void
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::CONST
$this->createContext($occursAtLine, DependencyType::CONST),
);
}

public function anonymousClassImplements(string $classLikeName, int $occursAtLine): void
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::ANONYMOUS_CLASS_IMPLEMENTS
$this->createContext($occursAtLine, DependencyType::ANONYMOUS_CLASS_IMPLEMENTS),
);
}

public function parameter(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::PARAMETER
$this->createContext($occursAtLine, DependencyType::PARAMETER),
);

return $this;
Expand All @@ -139,8 +135,7 @@ public function attribute(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::ATTRIBUTE
$this->createContext($occursAtLine, DependencyType::ATTRIBUTE),
);

return $this;
Expand All @@ -150,8 +145,7 @@ public function instanceof(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::INSTANCEOF
$this->createContext($occursAtLine, DependencyType::INSTANCEOF),
);

return $this;
Expand All @@ -161,8 +155,7 @@ public function newStatement(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::NEW
$this->createContext($occursAtLine, DependencyType::NEW),
);

return $this;
Expand All @@ -172,8 +165,7 @@ public function staticProperty(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::STATIC_PROPERTY
$this->createContext($occursAtLine, DependencyType::STATIC_PROPERTY),
);

return $this;
Expand All @@ -183,8 +175,7 @@ public function staticMethod(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::STATIC_METHOD
$this->createContext($occursAtLine, DependencyType::STATIC_METHOD),
);

return $this;
Expand All @@ -194,8 +185,7 @@ public function catchStmt(string $classLikeName, int $occursAtLine): self
{
$this->dependencies[] = new DependencyToken(
ClassLikeToken::fromFQCN($classLikeName),
new FileOccurrence($this->filepath, $occursAtLine),
DependencyType::CATCH
$this->createContext($occursAtLine, DependencyType::CATCH),
);

return $this;
Expand Down
2 changes: 2 additions & 0 deletions src/Core/Ast/Parser/Cache/AstFileReferenceFileCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Qossmic\Deptrac\Core\Ast\Parser\Cache;

use Qossmic\Deptrac\Contract\Ast\DependencyContext;
use Qossmic\Deptrac\Contract\Ast\DependencyType;
use Qossmic\Deptrac\Contract\Ast\FileOccurrence;
use Qossmic\Deptrac\Core\Ast\AstMap\AstInherit;
Expand Down Expand Up @@ -121,6 +122,7 @@ static function (array $data): array {
FunctionToken::class,
SuperGlobalToken::class,
FileOccurrence::class,
DependencyContext::class,
],
]
);
Expand Down
17 changes: 5 additions & 12 deletions src/Core/Dependency/Dependency.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@

namespace Qossmic\Deptrac\Core\Dependency;

use Qossmic\Deptrac\Contract\Ast\DependencyType;
use Qossmic\Deptrac\Contract\Ast\FileOccurrence;
use Qossmic\Deptrac\Contract\Ast\DependencyContext;
use Qossmic\Deptrac\Contract\Ast\TokenInterface;
use Qossmic\Deptrac\Contract\Dependency\DependencyInterface;

Expand All @@ -14,15 +13,14 @@ class Dependency implements DependencyInterface
public function __construct(
private readonly TokenInterface $depender,
private readonly TokenInterface $dependent,
private readonly FileOccurrence $fileOccurrence,
private readonly DependencyType $dependencyType
private readonly DependencyContext $context,
) {}

public function serialize(): array
{
return [[
'name' => $this->dependent->toString(),
'line' => $this->fileOccurrence->line,
'line' => $this->context->fileOccurrence->line,
]];
}

Expand All @@ -36,13 +34,8 @@ public function getDependent(): TokenInterface
return $this->dependent;
}

public function getFileOccurrence(): FileOccurrence
public function getContext(): DependencyContext
{
return $this->fileOccurrence;
}

public function getType(): DependencyType
{
return $this->dependencyType;
return $this->context;
}
}
11 changes: 5 additions & 6 deletions src/Core/Dependency/Emitter/ClassDependencyEmitter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Qossmic\Deptrac\Core\Dependency\Emitter;

use Qossmic\Deptrac\Contract\Ast\DependencyContext;
use Qossmic\Deptrac\Contract\Ast\DependencyType;
use Qossmic\Deptrac\Core\Ast\AstMap\AstMap;
use Qossmic\Deptrac\Core\Dependency\Dependency;
Expand All @@ -22,19 +23,18 @@ public function applyDependencies(AstMap $astMap, DependencyList $dependencyList
$classLikeName = $classReference->getToken();

foreach ($classReference->dependencies as $dependency) {
if (DependencyType::SUPERGLOBAL_VARIABLE === $dependency->type) {
if (DependencyType::SUPERGLOBAL_VARIABLE === $dependency->context->dependencyType) {
continue;
}
if (DependencyType::UNRESOLVED_FUNCTION_CALL === $dependency->type) {
if (DependencyType::UNRESOLVED_FUNCTION_CALL === $dependency->context->dependencyType) {
continue;
}

$dependencyList->addDependency(
new Dependency(
$classLikeName,
$dependency->token,
$dependency->fileOccurrence,
$dependency->type
$dependency->context,
)
);
}
Expand All @@ -44,8 +44,7 @@ public function applyDependencies(AstMap $astMap, DependencyList $dependencyList
new Dependency(
$classLikeName,
$inherit->classLikeName,
$inherit->fileOccurrence,
DependencyType::INHERIT
new DependencyContext($inherit->fileOccurrence, DependencyType::INHERIT),
)
);
}
Expand Down
5 changes: 2 additions & 3 deletions src/Core/Dependency/Emitter/ClassSuperglobalDependencyEmitter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@ public function applyDependencies(AstMap $astMap, DependencyList $dependencyList
{
foreach ($astMap->getClassLikeReferences() as $classReference) {
foreach ($classReference->dependencies as $dependency) {
if (DependencyType::SUPERGLOBAL_VARIABLE !== $dependency->type) {
if (DependencyType::SUPERGLOBAL_VARIABLE !== $dependency->context->dependencyType) {
continue;
}
$dependencyList->addDependency(
new Dependency(
$classReference->getToken(),
$dependency->token,
$dependency->fileOccurrence,
$dependency->type
$dependency->context,
)
);
}
Expand Down
Loading
Loading