Features/#462 gutter icons module case #465
Conversation
* Use Psi improoved * LineMarkerProvider was implemented for rule inheritance. Resolves: #429 Co-authored-by: Roman Chernyatchik <[email protected]>
| /** | ||
| * Stub of rule like element which inherits at least one rule | ||
| */ | ||
| interface RuleDescendantStub<T : PsiNamedElement> : NamedStub<T> { |
There was a problem hiding this comment.
Seems it is over-design. We have only one object that could inherit rules (use) and do not have any expectations that there will be more rules with such feature. So intermediate classes and interfaces RuleDescendantStub and RuleDescendantStubImpl have only single implementations and seems to be redundant. I suggest to leave only SmkUseStub and SmkUseStubImpl.
P.S SmkRuleLikeStubImpl is a different case, it hase 4 subclasses.
|
|
||
| override fun createStub(name: String?, parentStub: StubElement<*>?): SmkUseStub = | ||
| SmkUseStubImpl(name, parentStub) | ||
| lateinit var psi: SmkUse |
There was a problem hiding this comment.
What is the purpose of this field? Seems no usages + I consider it to be a nasty hack. It is factory, it shouldn't memorize reference to some of objects created via this factory.
| createStub(psi.name, parentStub) | ||
| createStub( | ||
| psi.name, | ||
| psi.getDefinedReferencesOfImportedRuleNames()?.map { it.text } ?: listOf( |
There was a problem hiding this comment.
get * reference has another meaning, normally reference that is associated with psi element, e.g. PsiElement.getReferences(): PsiRference[]. Please use less confusing naming here. Also I think better will be do not break incapsulation idea of OOP and do smth like use.getImportedNamesList():SmkImportedRulesNamesList? and SmkImportedRulesNamesList.arguments(): SmkReferenceExpression[]
There was a problem hiding this comment.
SmkRuleDescendantStub.getInheritedRulesNames() seems should be also method of SmkImportedRulesNamesList.arguments stub.
| // We don't save it in stub because it requires 'resolve' | ||
| // We compare resolve results even for descendantsDefinedExplicitly | ||
| // Because there may be rules with the same names | ||
| if (descendant.getImportedRules()?.contains(element) == true) { |
There was a problem hiding this comment.
getImportedRules() name sounds to innocent, like some simple filter on AST nodes, similar to other methods. Actually it is heavy method that does resolve. Let's emphasize that it isn't lightweight method and does resolve, e.g name it use.getImportedRules().resolveNames()
…ub.com/JetBrains-Research/snakecharm into features/#462-gutter-icons-module-case
…ub.com/JetBrains-Research/snakecharm into features/#462-gutter-icons-module-case � Conflicts: � CHANGELOG.md � src/main/kotlin/com/jetbrains/snakecharm/SnakemakeRuleInheritanceMarkerProvider.kt � src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkIgnorePyInspectionExtension.kt � src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkNotSameWildcardsSetInspection.kt � src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkSeveralRulesAreOverriddenAsOneInspection.kt � src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkUnresolvedImportedRuleNameInspection.kt � src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkUnusedLogFileInspection.kt � src/main/kotlin/com/jetbrains/snakecharm/lang/parser/SmkStatementParsing.kt � src/main/kotlin/com/jetbrains/snakecharm/lang/psi/SmkPsiElements.kt � src/main/kotlin/com/jetbrains/snakecharm/lang/psi/SmkWildcardsCollector.kt � src/main/kotlin/com/jetbrains/snakecharm/lang/psi/elementTypes/SmkElementTypes.kt � src/main/kotlin/com/jetbrains/snakecharm/lang/psi/impl/SmkUseImpl.kt � src/main/kotlin/com/jetbrains/snakecharm/lang/psi/impl/stubs/SmkUseElementType.kt � src/main/kotlin/com/jetbrains/snakecharm/lang/psi/types/SmkWildcardsType.kt � testData/psi/Use.txt � testData/psi/UseIncomplete.txt � testData/psi/UseInvalid.txt
Resolves: #462
|
API was cleaned-up |
New index was implemented, so issues #462 and #463 were resolved