Skip to content

Commit

Permalink
fixed duplicate bug in trait inspection apply
Browse files Browse the repository at this point in the history
added new test from kphp
moved
  • Loading branch information
Hidanio committed Oct 24, 2023
1 parent 405e3ae commit ffabdd5
Show file tree
Hide file tree
Showing 13 changed files with 85 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import com.jetbrains.php.lang.psi.visitors.PhpElementVisitor
import com.vk.modulite.SymbolName
import com.vk.modulite.composer.ComposerPackage
import com.vk.modulite.modulite.Modulite
import com.vk.modulite.modulite.ModuliteRequires
import com.vk.modulite.modulite.ModuliteRestrictionChecker
import com.vk.modulite.psi.extensions.files.containingComposerPackage
import com.vk.modulite.psi.extensions.files.containingModulite
Expand Down Expand Up @@ -240,7 +241,7 @@ class InternalSymbolUsageInspection : LocalInspectionTool() {
restricted to $readableName, $refModulite is not required by ${context.modulite}
""".trimIndent()
} else if(symbolElement is PhpClass && symbolElement.isTrait){
val (classes,methods) = collectTraitReferenceUsage(reference)
val (classes,methods) = collectTraitReferenceUsage(reference,context.modulite?.requires )
quickFixes.add(AddSymbolToRequiresQuickFix(context.modulite!!, classes+methods))

"""
Expand All @@ -265,7 +266,7 @@ class InternalSymbolUsageInspection : LocalInspectionTool() {
""".trimIndent()
}
}

context.modulite?.requires
registerModuliteProblem(
problemElement,
text,
Expand Down Expand Up @@ -314,7 +315,7 @@ class InternalSymbolUsageInspection : LocalInspectionTool() {
return Pair(classesToRequire, methodsToRequire)
}

private fun collectTraitReferenceUsage(reference: PhpReference)
private fun collectTraitReferenceUsage(reference: PhpReference, moduliteRequires : ModuliteRequires? = null)
: Pair<List<SymbolName>, List<SymbolName>> {
val traitsClasses: MutableList<PhpClass> = arrayListOf()
val methodsNames: MutableCollection<Method> = arrayListOf()
Expand Down Expand Up @@ -361,9 +362,15 @@ class InternalSymbolUsageInspection : LocalInspectionTool() {
requireMethods.addAll(methods)
}

val traitsSymbolsName = traitsClasses.distinct().mapNotNull { processElement(it, reference) }
val traitsSymbolName = requireMethods.distinct().mapNotNull { processElement(it, reference) }
val traitsClassName = traitsClasses.distinct().mapNotNull { processElement(it, reference) }
val traitsMethodsName = requireMethods.distinct().mapNotNull { processElement(it, reference) }

moduliteRequires?.symbols?.let { currentModuleSymbols ->
val filteredMethods = traitsMethodsName.filterNot { it in currentModuleSymbols }
val filteredClasses = traitsClassName.filterNot { it in currentModuleSymbols }
return Pair(filteredClasses, filteredMethods)
}

return Pair(traitsSymbolsName, traitsSymbolName)
return Pair(traitsClassName, traitsMethodsName)
}
}
11 changes: 1 addition & 10 deletions src/test/fixtures/golden/TraitsNesting/Photos/.modulite.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,8 @@ require:
- "\\Engines\\LikesEngineTrait"
- "\\Engines\\MusicEngineTrait"
- "\\Engines\\VideosEngineTrait"
- "\\Engines\\FriendsEngineTrait::friendList()"
- "\\Engines\\FriendsEngineTrait::printFriendList()"
- "\\Engines\\LikesEngineTrait::print()"
- "\\Engines\\LikesEngineTrait::printTweetsLikes()"
- "\\Engines\\MusicEngineTrait::musicList()"
- "\\Engines\\MusicEngineTrait::musicListPrint()"
- "\\ExternalEngines\\Tweets::printTweetsLikesCountByUser()"
- "\\Engines\\TestEngine::tester()"
- "\\Engines\\VideosEngineTrait::printVideosList()"
- "\\Engines\\VideosEngineTrait::videosList()"



# Granting partial access to internal symbols, "as an exception".
allow-internal-access:
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@
require_once 'kphp_tester_include.php';
#endif

class MMM101 {
use Feed101\PrivTrait;
// ^^^^^^^^^^^^^^^^^
// error: restricted to use Feed101\PrivTrait, it's internal in @feed
class MMM006 {
use Feed006\PrivTrait;
}
(new MMM101)->showThisClass();
(new MMM006)->showThisClass();

// here $impl is auto-deduced as PrivClass because of typed callable
// so, we can call its instance methods, since we don't analyze instance methods
// but we can't write a type hint or extract this callback into another var, as we can't use that internal class in phpdoc
Feed101\PubFeed::accessToPrivClass(function($impl) { // function(PrivClass $impl) is erroneous
Feed006\PubFeed::accessToPrivClass(function($impl) { // function(PrivClass $impl) is erroneous
$impl->myMethod();
});
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
name: "@feed"
namespace: "\\Feed101"
namespace: "\\Feed006"

export:
- "PubFeed"
- "PrivTrait"

force-internal:

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Feed101;
namespace Feed006;

class PrivClass {
public function myMethod() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Feed101;
namespace Feed006;

trait PrivTrait {
function showThisClass() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Feed101;
namespace Feed006;

class PubFeed {
/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
@ok
<?php
#ifndef KPHP
require_once 'kphp_tester_include.php';
#endif

\Photos101\Photo101::printPhotoLikes();
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace Engines101;

trait LikesEngineTrait101 {
public static function print(): void {
echo "like";
}

public static function printTweetsLikes(): void {
\ExternalEngines101\Tweets101::printTweetsLikesCountByUser();
$_ = new \ExternalEngines101\Tweets101();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

namespace ExternalEngines101;

class Tweets101 {
public static function printTweetsLikesCountByUser() {
echo "ExternalEngines: tweets like for this user";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: "@photos"
description: ""
namespace: "Photos101\\"

# "Public API" of the modulite: classes, functions, constants, etc.
# Symbols not listed here will be internal.
export:
- "Photo101"

# Class members to exclude, they override "export".
force-internal:

# Dependencies: other modulites, global classes, defines, etc.
require:

# Granting partial access to internal symbols, "as an exception".
allow-internal-access:
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace Photos101;

class Photo101 {
use \Engines101\LikesEngineTrait101;
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//error: restricted to use Engines101\LikesEngineTrait101, it's not required by @photos
public static function printPhotoLikes() {
self::print();
}
}
4 changes: 3 additions & 1 deletion src/test/kotlin/com/vk/modulite/tests/KphpGoldenTests.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class KphpGoldenTests : ModuliteInspectionTestBase() {
fun `test 003_allow_internal`() = runFixture("kphp_golden/003_allow_internal")
fun `test 004_instance_methods`() = runFixture("kphp_golden/004_instance_methods")
fun `test 005_inheritance`() = runFixture("kphp_golden/005_inheritance")
fun `test 006_known_bugs`() = runFixture("kphp_golden/006_known_bugs")
fun `test 008_mod_generics`() = runFixture("kphp_golden/008_mod_generics")
fun `test 009_mod_magic_m`() = runFixture("kphp_golden/009_mod_magic_m")
fun `test 010_mod_unreachable`() = runFixture("kphp_golden/010_mod_unreachable")
Expand All @@ -20,5 +21,6 @@ class KphpGoldenTests : ModuliteInspectionTestBase() {
//fun `test 007_composer_ok`() = runFixture("kphp_golden/007_composer_ok")

fun `test 100_wrong_static_bindings`() = runFixture("kphp_golden/100_wrong_static_bindings")
fun `test 101_trait_in_module_internal`() = runFixture("kphp_golden/101_trait_in_module_internal")
fun `test 101_trait_not_required`() = runFixture("kphp_golden/101_trait_not_required")

}

0 comments on commit ffabdd5

Please sign in to comment.