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

Check interfaces that extend other interfaces. #120

Merged
merged 1 commit into from
Jul 10, 2024
+52 −69
Merged

Check interfaces that extend other interfaces. #120

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 35 additions & 69 deletions src/Checks/InterfaceCheck.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use PhpParser\Node;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassLike;
use PHPUnit\Util\Type;
use PhpParser\Node\Stmt\Interface_;

/**
* Class InterfaceCheck
Expand Down Expand Up @@ -50,7 +50,7 @@ function __construct(SymbolTable $symbolTable, OutputInterface $doc) {
* @return array
*/
public function getCheckNodeTypes() {
return [Class_::class];
return [Class_::class, Interface_::class];
}

/**
Expand All @@ -65,7 +65,7 @@ public function getCheckNodeTypes() {
*
* @return void
*/
protected function checkMethod($fileName, Class_ $class, Node\FunctionLike $astNode, MethodInterface $method, MethodInterface $parentMethod) {
protected function checkMethod($fileName, Class_|Interface_ $class, Node\FunctionLike $astNode, MethodInterface $method, MethodInterface $parentMethod) {

$visibility = $method->getAccessLevel();
$oldVisibility = $parentMethod->getAccessLevel();
Expand Down Expand Up @@ -96,7 +96,7 @@ protected function checkMethod($fileName, Class_ $class, Node\FunctionLike $astN
if (!$isContravariant) {
$childParamType = TypeComparer::typeToString($childParam->getType());
$parentParamType = TypeComparer::typeToString($parentParam->getType());
$this->emitErrorOnLine($fileName, $method->getStartingLine(), self::TYPE_SIGNATURE_TYPE, "Child method parameter " . $childParam->getName() . " type mismatch " . $className . "::" . $method->getName() . " : $parentParamType -> $childParamType");
$this->emitError($fileName, $astNode, self::TYPE_SIGNATURE_TYPE, "Child method parameter " . $childParam->getName() . " type mismatch " . $className . "::" . $method->getName() . " : $parentParamType -> $childParamType");
}
}
if ($childParam->getName() != $parentParam->getName()) {
Expand Down Expand Up @@ -163,34 +163,6 @@ private function assertParentChildReturnTypesMatch(
}
}

/**
* implementsMethod
*
* @param Class_ $node Instance of ClassAbstraction
* @param string $interfaceMethod The interface
*
* @return ClassMethod|null
*/
protected function implementsMethod(Class_ $node, $interfaceMethod) {
$current = new AbstractedClass_($node);
while (true) {
// Is it directly in the class
$classMethod = $current->getMethod($interfaceMethod);
if ($classMethod) {
return $classMethod;
}

if ($current->getParentClassName()) {
$current = $this->symbolTable->getAbstractedClass($current->getParentClassName());
if (!$current) {
return null;
}
} else {
return null;
}
}
}

/**
* run
*
Expand All @@ -203,13 +175,16 @@ protected function implementsMethod(Class_ $node, $interfaceMethod) {
*/
public function run($fileName, Node $node, ClassLike $inside = null, Scope $scope = null) {
if ($node instanceof Class_) {
if ($node->implements) {
if ($node->implements || $node->extends) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the meat of the change. If a class implements OR extends any class, then we build a deep list of interfaces and confirm that you implement all the methods.

$this->processNodeImplements($fileName, $node);
}
if ($node->extends) {
$this->processNodeExtends($fileName, $node);
}
}
if ($node instanceof Interface_ && $node->extends) {
$this->processNodeImplements($fileName, $node);
}
Copy link
Contributor Author

@jongardiner jongardiner Jul 10, 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 a new check. If you have one interface that extends other interfaces, then we make sure that any common methods have compatible signatures.

}

/**
Expand All @@ -220,30 +195,19 @@ public function run($fileName, Node $node, ClassLike $inside = null, Scope $scop
*
* @return void
*/
private function processNodeImplements($fileName, Class_ $node) {
$arr = is_array($node->implements) ? $node->implements : [$node->implements];
foreach ($arr as $interface) {
$name = $interface->toString();
if ($name) {
$interface = $this->symbolTable->getAbstractedClass($name);
if (!$interface) {
$this->emitError($fileName, $node, ErrorConstants::TYPE_UNKNOWN_CLASS, $node->name . " implements unknown interface " . $name);
} else {
$this->processNodeImplementsNotAbstract($fileName, $node, $interface);
}
private function processNodeImplements(string $fileName, Class_|Interface_ $node):void {
$arr = Util::findAllInterfaces(strval($node->namespacedName), $this->symbolTable);
foreach ($arr as $name) {
$interface = $this->symbolTable->getAbstractedClass($name);
if (!$interface) {
$this->emitError($fileName, $node, ErrorConstants::TYPE_UNKNOWN_CLASS, $node->name . " implements unknown interface " . $name);
} else {
$this->processNodeImplementsInterface($fileName, $node, $interface);
}
}
}

/**
* processNodeExtends
*
* @param string $fileName The file name
* @param Node $node Instance of Node
*
* @return void
*/
private function processNodeExtends($fileName, Class_ $node) {
private function processNodeExtends(string $fileName, Class_ $node):void {
$class = new AbstractedClass_($node);
$parentClass = $this->symbolTable->getAbstractedClass($node->extends);
if (!$parentClass) {
Expand All @@ -262,22 +226,24 @@ private function processNodeExtends($fileName, Class_ $node) {
}
}

private function processNodeImplementsNotAbstract($fileName, Class_ $node, ClassInterface $interface) {
// Don't force abstract classes to implement all methods.
if (!$node->isAbstract()) {
foreach ($interface->getMethodNames() as $interfaceMethod) {
$classMethod = $this->implementsMethod($node, $interfaceMethod);
if (!$classMethod) {
if (!$node->isAbstract()) {
$this->emitError($fileName, $node, ErrorConstants::TYPE_UNIMPLEMENTED_METHOD, $node->name . " does not implement method " . $interfaceMethod);
}
} else {
foreach($node->stmts as $stmt) {
if ($stmt instanceof Node\Stmt\ClassMethod && strcasecmp($stmt->name, $interfaceMethod)==0) {
$this->checkMethod($fileName, $node, $stmt, $classMethod, $interface->getMethod($interfaceMethod));
break;
}
}
private function processNodeImplementsInterface(string $fileName, Class_|Interface_ $node, ClassInterface $interface):void {
$methods = $interface->getMethodNames();
foreach($methods as $methodName) {
$this->processInterfaceMethod($node, $methodName, $fileName, $interface);
}
}

public function processInterfaceMethod(Class_|Interface_ $node, string $interfaceMethod, string $fileName, ClassInterface $interface) : void {
$classMethod = Util::findAbstractedMethod($node->namespacedName, $interfaceMethod, $this->symbolTable);
if (!$classMethod) {
if ($node instanceof Node\Stmt\Class_ && !$node->isAbstract()) {
$this->emitError($fileName, $node, ErrorConstants::TYPE_UNIMPLEMENTED_METHOD, $node->name . " does not implement method " . $interfaceMethod);
}
} else {
foreach ($node->stmts as $stmt) {
if ($stmt instanceof Node\Stmt\ClassMethod && strcasecmp($stmt->name, $interfaceMethod) == 0) {
$this->checkMethod($fileName, $node, $stmt, $classMethod, $interface->getMethod($interfaceMethod));
break;
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions src/Util.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,23 @@ static public function findAbstractedMethod($className, $name, SymbolTable $symb
return null;
}

static public function findAllInterfaces(string $className, SymbolTable $symbolTable):array {
$interfaces = [];
while ($className) {
$class = $symbolTable->getAbstractedClass($className);
if (!$class) {
return $interfaces;
}
$immediateList = $class->getInterfaceNames();
foreach($immediateList as $immediate ) {
$childInterfaces = static::findAllInterfaces($immediate, $symbolTable);
$interfaces = [ ...$childInterfaces, $immediate, ...$interfaces];
}
$className = $class->getParentClassName();
}
return array_unique($interfaces);
}

static function findAbstractedConstantExpr(string $className, string $constantName, SymbolTable $symbolTable) {
while ($className) {
$class = $symbolTable->getAbstractedClass($className);
Expand Down
Loading