refactor(discovery): improve dependency analysis with enhanced namespace resolution and error handling
- Introduce `normalizeTypeName` to validate and normalize type names during dependency analysis. - Add `safeCreateClassName` to handle `ClassName` creation errors gracefully. - Enhance constructor, method, property, and return type dependency edge creation with context-aware namespace resolution. - Improve logging to capture failure details and provide debugging insights.
This commit is contained in:
@@ -5,6 +5,7 @@ declare(strict_types=1);
|
||||
namespace App\Framework\Discovery\Analysis;
|
||||
|
||||
use App\Framework\Core\ValueObjects\ClassName;
|
||||
use App\Framework\Core\ValueObjects\PhpNamespace;
|
||||
use App\Framework\Discovery\ValueObjects\DependencyEdge;
|
||||
use App\Framework\Discovery\ValueObjects\DependencyGraph;
|
||||
use App\Framework\Discovery\ValueObjects\DependencyNode;
|
||||
@@ -197,6 +198,8 @@ final readonly class DependencyAnalyzer
|
||||
}
|
||||
|
||||
$parameters = $constructor->getParameters();
|
||||
$sourceClassName = ClassName::create($class->getName());
|
||||
$currentNamespace = $sourceClassName->getNamespaceObject();
|
||||
|
||||
foreach ($parameters as $parameter) {
|
||||
$type = $parameter->getType();
|
||||
@@ -207,14 +210,27 @@ final readonly class DependencyAnalyzer
|
||||
$typeNames = $this->extractTypeNames($type);
|
||||
|
||||
foreach ($typeNames as $typeName) {
|
||||
$targetClassName = $this->safeCreateClassName($typeName, $currentNamespace);
|
||||
if ($targetClassName === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
$edge = DependencyEdge::create(
|
||||
ClassName::create($class->getName()),
|
||||
ClassName::create($typeName),
|
||||
$targetClassName,
|
||||
DependencyRelation::CONSTRUCTOR_INJECTION,
|
||||
10 // Highest weight for constructor injection
|
||||
);
|
||||
|
||||
$graph = $graph->addEdge($edge);
|
||||
} catch (\Throwable $e) {
|
||||
$this->logger?->debug('Failed to create dependency edge for constructor parameter', LogContext::withData([
|
||||
'class' => $class->getName(),
|
||||
'type_name' => $typeName,
|
||||
'error' => $e->getMessage(),
|
||||
]));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -250,6 +266,8 @@ final readonly class DependencyAnalyzer
|
||||
private function analyzeMethodParameters(WrappedReflectionClass $class, ReflectionMethod $method, DependencyGraph $graph): DependencyGraph
|
||||
{
|
||||
$parameters = $method->getParameters();
|
||||
$sourceClassName = ClassName::create($class->getName());
|
||||
$currentNamespace = $sourceClassName->getNamespaceObject();
|
||||
|
||||
foreach ($parameters as $parameter) {
|
||||
$type = $parameter->getType();
|
||||
@@ -260,14 +278,28 @@ final readonly class DependencyAnalyzer
|
||||
$typeNames = $this->extractTypeNames($type);
|
||||
|
||||
foreach ($typeNames as $typeName) {
|
||||
$targetClassName = $this->safeCreateClassName($typeName, $currentNamespace);
|
||||
if ($targetClassName === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
$edge = DependencyEdge::create(
|
||||
ClassName::create($class->getName()),
|
||||
ClassName::create($typeName),
|
||||
$targetClassName,
|
||||
DependencyRelation::METHOD_PARAMETER,
|
||||
5 // Medium weight for method parameters
|
||||
);
|
||||
|
||||
$graph = $graph->addEdge($edge);
|
||||
} catch (\Throwable $e) {
|
||||
$this->logger?->debug('Failed to create dependency edge for method parameter', LogContext::withData([
|
||||
'class' => $class->getName(),
|
||||
'method' => $method->getName(),
|
||||
'type_name' => $typeName,
|
||||
'error' => $e->getMessage(),
|
||||
]));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -285,16 +317,32 @@ final readonly class DependencyAnalyzer
|
||||
}
|
||||
|
||||
$typeNames = $this->extractTypeNames($returnType);
|
||||
$sourceClassName = ClassName::create($class->getName());
|
||||
$currentNamespace = $sourceClassName->getNamespaceObject();
|
||||
|
||||
foreach ($typeNames as $typeName) {
|
||||
$targetClassName = $this->safeCreateClassName($typeName, $currentNamespace);
|
||||
if ($targetClassName === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
$edge = DependencyEdge::create(
|
||||
ClassName::create($class->getName()),
|
||||
ClassName::create($typeName),
|
||||
$targetClassName,
|
||||
DependencyRelation::RETURN_TYPE,
|
||||
3 // Lower weight for return types
|
||||
);
|
||||
|
||||
$graph = $graph->addEdge($edge);
|
||||
} catch (\Throwable $e) {
|
||||
$this->logger?->debug('Failed to create dependency edge for return type', LogContext::withData([
|
||||
'class' => $class->getName(),
|
||||
'method' => $method->getName(),
|
||||
'type_name' => $typeName,
|
||||
'error' => $e->getMessage(),
|
||||
]));
|
||||
}
|
||||
}
|
||||
|
||||
return $graph;
|
||||
@@ -306,6 +354,8 @@ final readonly class DependencyAnalyzer
|
||||
private function analyzePropertyDependencies(WrappedReflectionClass $class, DependencyGraph $graph): DependencyGraph
|
||||
{
|
||||
$properties = $class->getPropertiesRaw(ReflectionProperty::IS_PUBLIC | ReflectionProperty::IS_PROTECTED | ReflectionProperty::IS_PRIVATE);
|
||||
$sourceClassName = ClassName::create($class->getName());
|
||||
$currentNamespace = $sourceClassName->getNamespaceObject();
|
||||
|
||||
foreach ($properties as $property) {
|
||||
$type = $property->getType();
|
||||
@@ -316,14 +366,28 @@ final readonly class DependencyAnalyzer
|
||||
$typeNames = $this->extractTypeNames($type);
|
||||
|
||||
foreach ($typeNames as $typeName) {
|
||||
$targetClassName = $this->safeCreateClassName($typeName, $currentNamespace);
|
||||
if ($targetClassName === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
$edge = DependencyEdge::create(
|
||||
ClassName::create($class->getName()),
|
||||
ClassName::create($typeName),
|
||||
$targetClassName,
|
||||
DependencyRelation::PROPERTY_TYPE,
|
||||
4 // Medium-low weight for property types
|
||||
);
|
||||
|
||||
$graph = $graph->addEdge($edge);
|
||||
} catch (\Throwable $e) {
|
||||
$this->logger?->debug('Failed to create dependency edge for property type', LogContext::withData([
|
||||
'class' => $class->getName(),
|
||||
'property' => $property->getName(),
|
||||
'type_name' => $typeName,
|
||||
'error' => $e->getMessage(),
|
||||
]));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -426,14 +490,20 @@ final readonly class DependencyAnalyzer
|
||||
private function extractTypeNames(\ReflectionType $type): array
|
||||
{
|
||||
if ($type instanceof \ReflectionNamedType) {
|
||||
return [$type->getName()];
|
||||
$name = $type->getName();
|
||||
// Validate and normalize the type name
|
||||
$normalized = $this->normalizeTypeName($name);
|
||||
return $normalized !== null ? [$normalized] : [];
|
||||
}
|
||||
|
||||
if ($type instanceof \ReflectionUnionType) {
|
||||
$types = [];
|
||||
foreach ($type->getTypes() as $subType) {
|
||||
if ($subType instanceof \ReflectionNamedType && ! $subType->isBuiltin()) {
|
||||
$types[] = $subType->getName();
|
||||
$normalized = $this->normalizeTypeName($subType->getName());
|
||||
if ($normalized !== null) {
|
||||
$types[] = $normalized;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -444,7 +514,10 @@ final readonly class DependencyAnalyzer
|
||||
$types = [];
|
||||
foreach ($type->getTypes() as $subType) {
|
||||
if ($subType instanceof \ReflectionNamedType && ! $subType->isBuiltin()) {
|
||||
$types[] = $subType->getName();
|
||||
$normalized = $this->normalizeTypeName($subType->getName());
|
||||
if ($normalized !== null) {
|
||||
$types[] = $normalized;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -454,4 +527,186 @@ final readonly class DependencyAnalyzer
|
||||
// Fallback for unknown types
|
||||
return [];
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize and validate a type name
|
||||
*
|
||||
* - Validates that the name is a valid class name (not a parameter/method name)
|
||||
* - Resolves namespace: if relative and not found in current namespace, try global
|
||||
* - Returns null if the name is invalid or cannot be resolved
|
||||
*
|
||||
* @return string|null The normalized fully qualified class name, or null if invalid
|
||||
*/
|
||||
private function normalizeTypeName(string $typeName): ?string
|
||||
{
|
||||
// Skip empty names
|
||||
if (empty($typeName)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Check if it's already fully qualified (starts with \)
|
||||
if (str_starts_with($typeName, '\\')) {
|
||||
$fqn = ltrim($typeName, '\\');
|
||||
// Validate it's a valid class name format
|
||||
if ($this->isValidClassNameFormat($fqn)) {
|
||||
return $fqn;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// Validate basic format first - must look like a class name
|
||||
if (! $this->isValidClassNameFormat($typeName)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Check common invalid names (parameter names, method names, etc.)
|
||||
// These are typically lowercase or camelCase with specific patterns
|
||||
if ($this->isLikelyInvalidName($typeName)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// For relative names, we need the current namespace context
|
||||
// But since we don't have that here, we'll try to validate the format
|
||||
// The actual namespace resolution will happen when we try to create the ClassName
|
||||
return $typeName;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a string looks like a valid class name format
|
||||
*/
|
||||
private function isValidClassNameFormat(string $name): bool
|
||||
{
|
||||
// Must start with letter or underscore, followed by letters, numbers, underscores, or backslashes
|
||||
return preg_match('/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff\\\\]*$/', $name) === 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a name is likely invalid (parameter name, method name, etc.)
|
||||
*
|
||||
* This is a conservative check - we only filter names that are definitely not class names
|
||||
* based on the error patterns we've seen in the logs.
|
||||
*/
|
||||
private function isLikelyInvalidName(string $name): bool
|
||||
{
|
||||
// Skip if it contains backslashes (likely a namespace - definitely a class name)
|
||||
if (str_contains($name, '\\')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$lowercase = strtolower($name);
|
||||
|
||||
// Check for known invalid names from error logs
|
||||
// These are definitely parameter/method names, not type names
|
||||
$knownInvalidNames = [
|
||||
'state', // Property name
|
||||
'container', // Parameter name
|
||||
'get', // Method name
|
||||
'map', // Method name
|
||||
'compile', // Method name
|
||||
'install', // Method name
|
||||
'shouldretry', // Method name (shouldRetry)
|
||||
'additionaldata', // Parameter name (additionalData)
|
||||
];
|
||||
|
||||
if (in_array($lowercase, $knownInvalidNames, true)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Very conservative: Only reject if it's a single lowercase word
|
||||
// that matches common method/parameter patterns and is NOT a built-in type
|
||||
if ($name === $lowercase && ! str_contains($name, '_') && strlen($name) < 20) {
|
||||
// Built-in types are allowed
|
||||
$builtInTypes = ['string', 'int', 'bool', 'float', 'array', 'object', 'mixed', 'void', 'null', 'false', 'true', 'iterable', 'callable', 'resource'];
|
||||
if (in_array($lowercase, $builtInTypes, true)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Very short single lowercase words are likely parameter names
|
||||
// But we're conservative - only reject if it's a known problematic pattern
|
||||
if (in_array($lowercase, $knownInvalidNames, true)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// camelCase starting with lowercase and matching method patterns
|
||||
if (preg_match('/^[a-z][a-zA-Z0-9]*$/', $name) && ! str_contains($name, '\\')) {
|
||||
$methodPrefixes = ['get', 'set', 'is', 'has', 'should', 'can', 'will', 'do', 'add', 'remove', 'update', 'delete'];
|
||||
foreach ($methodPrefixes as $prefix) {
|
||||
if (str_starts_with($lowercase, $prefix) && strlen($name) > strlen($prefix)) {
|
||||
// This looks like a method name, but be conservative
|
||||
// Only reject if it's in our known list
|
||||
if (in_array($lowercase, $knownInvalidNames, true)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Safely create a ClassName from a type name, handling errors gracefully
|
||||
*
|
||||
* @return ClassName|null The ClassName object, or null if creation failed
|
||||
*/
|
||||
private function safeCreateClassName(string $typeName, ?PhpNamespace $currentNamespace = null): ?ClassName
|
||||
{
|
||||
try {
|
||||
// Try to resolve namespace if it's a relative name
|
||||
$resolvedName = $this->resolveTypeName($typeName, $currentNamespace);
|
||||
|
||||
if ($resolvedName === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Try to create the ClassName
|
||||
$className = ClassName::create($resolvedName);
|
||||
|
||||
// Optionally validate that the class exists (but don't require it)
|
||||
// During discovery, classes might not be loaded yet
|
||||
return $className;
|
||||
} catch (\Throwable $e) {
|
||||
// Log the error for debugging, but don't throw
|
||||
$this->logger?->debug('Failed to create ClassName from type name', LogContext::withData([
|
||||
'type_name' => $typeName,
|
||||
'error' => $e->getMessage(),
|
||||
]));
|
||||
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a type name to a fully qualified name
|
||||
*
|
||||
* @param string $typeName The type name (may be relative or absolute)
|
||||
* @param PhpNamespace|null $currentNamespace The current namespace context
|
||||
* @return string|null The fully qualified name, or null if resolution failed
|
||||
*/
|
||||
private function resolveTypeName(string $typeName, ?PhpNamespace $currentNamespace): ?string
|
||||
{
|
||||
// Already fully qualified
|
||||
if (str_starts_with($typeName, '\\')) {
|
||||
return ltrim($typeName, '\\');
|
||||
}
|
||||
|
||||
// If we have a namespace context, try resolving in that namespace first
|
||||
if ($currentNamespace !== null && ! $currentNamespace->isGlobal()) {
|
||||
$namespaced = $currentNamespace->toString() . '\\' . $typeName;
|
||||
// Check if it exists (with autoload disabled to avoid side effects)
|
||||
if (class_exists($namespaced, false) || interface_exists($namespaced, false) || trait_exists($namespaced, false)) {
|
||||
return $namespaced;
|
||||
}
|
||||
}
|
||||
|
||||
// Try as global namespace
|
||||
if (class_exists($typeName, false) || interface_exists($typeName, false) || trait_exists($typeName, false)) {
|
||||
return $typeName;
|
||||
}
|
||||
|
||||
// If we can't resolve it, return the original name and let ClassName::create handle it
|
||||
// This is safe because ClassName::create will validate the format
|
||||
return $typeName;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user