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.

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

Features/#462 gutter icons module case #465

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
522b9f7
LineMarkerProvider was implemented for rule inheritance.
Nov 4, 2021
49fd4c6
Merge branch 'master' of https://github.com/JetBrains-Research/snakec…
Dec 30, 2021
ab65a0e
Now rule names are resolved properly.
Dec 30, 2021
fe4261a
Update CHANGELOG.md
dakochik Dec 30, 2021
b50ba5f
Tests were added. Minor fixes in marker provider and smk use implemen…
Jan 13, 2022
486e511
Merge branch 'features/#455-unresolved-rules' of https://github.com/J…
Jan 13, 2022
4510e23
Now rules which were imported via 'include' are supported too.
Jan 13, 2022
ae940f0
refactor: code cleanup
iromeo Jan 15, 2022
7c72c6d
Merge branch 'master' of https://github.com/JetBrains-Research/snakec…
Jan 17, 2022
83c436b
Code cleanup.
Jan 17, 2022
9c62dda
Tests fix.
Jan 22, 2022
6380fa3
New index for inherited rules was created. Behaviour of creating gutt…
Jan 29, 2022
fdbdba8
Misspelling in test was fixed
dakochik Jan 29, 2022
24663bb
feature: logos updated
iromeo Feb 4, 2022
13cde2f
refactor: file renamed
iromeo Feb 4, 2022
9a56970
fix: readme logo fixed
iromeo Feb 4, 2022
0551777
chore: libs updated
iromeo Feb 4, 2022
f26921d
fix: test execution from IDEA fixed
iromeo Feb 4, 2022
aeb8b22
fix: 1) cleanup 2) libs updated
iromeo Feb 4, 2022
262b914
Features/#452 quick fix in unused log file inspection (#453)
dakochik Feb 4, 2022
0b4f2b3
fix: Checkpoints could be accessed as rules when using rules #262
dakochik Feb 4, 2022
16a657f
fix: Show override gutter icon for use / overload rules #429
dakochik Feb 4, 2022
6f7b489
feat: Snakemake file type icon updated
iromeo Feb 10, 2022
7344c94
feat: 'copy-minimal' shadow section missing in completion #467
iromeo Feb 10, 2022
37d2024
API cleanup
Mar 28, 2022
0dc46cb
Merge branch 'features/#462-gutter-icons-module-case' of https://gith…
Mar 28, 2022
3676b79
API cleanup, rebase conflicts were resolved
Nov 4, 2021
baa4d7e
Merge branch 'features/#462-gutter-icons-module-case' of https://gith…
Mar 28, 2022
96f23b0
Conflicts resolving
Mar 28, 2022
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Released on ...
- TODO (see [#NNN](https://github.com/JetBrains-Research/snakecharm/issues/NNN))

### Fixed
- Complex cases in creation of gutter icons for rule inheritance (see [#462](https://github.com/JetBrains-Research/snakecharm/issues/462), [#463](https://github.com/JetBrains-Research/snakecharm/issues/463))
- Resolve for rule names in `use rule` section (see [#455](https://github.com/JetBrains-Research/snakecharm/issues/455))
- Multiple args inspection in `workdir` case (see [#140](https://github.com/JetBrains-Research/snakecharm/issues/140))
- `localrules` and `ruleorder` now take into account `use rule` (see [#448](https://github.com/JetBrains-Research/snakecharm/issues/448))
Expand All @@ -19,6 +20,7 @@ Released on ...

### Added
- Quick fix for unresolved files (conda, configfile. etc.) (see [#277](https://github.com/JetBrains-Research/snakecharm/issues/277))
- Gutter icons for rule inheritance (see [#429](https://github.com/JetBrains-Research/snakecharm/issues/429))
- Warning: Snakemake support is disabled (see [#109](https://github.com/JetBrains-Research/snakecharm/issues/109))
- TODO (see [#NNN](https://github.com/JetBrains-Research/snakecharm/issues/NNN))

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package com.jetbrains.snakecharm

import com.intellij.codeInsight.daemon.RelatedItemLineMarkerInfo
import com.intellij.codeInsight.daemon.RelatedItemLineMarkerProvider
import com.intellij.codeInsight.navigation.NavigationGutterIconBuilder
import com.intellij.icons.AllIcons
import com.intellij.openapi.module.ModuleUtilCore
import com.intellij.psi.PsiElement
import com.intellij.psi.search.GlobalSearchScope
import com.intellij.psi.stubs.StubIndex
import com.jetbrains.snakecharm.lang.psi.*
import com.jetbrains.snakecharm.lang.psi.stubs.SmkUseInheritedRulesIndex
import com.jetbrains.snakecharm.lang.psi.stubs.SmkUseInheritedRulesIndex.Companion.INHERITED_RULES_DECLARATION_VIA_WILDCARD

class SnakemakeRuleInheritanceMarkerProvider : RelatedItemLineMarkerProvider() {

override fun collectNavigationMarkers(
element: PsiElement,
result: MutableCollection<in RelatedItemLineMarkerInfo<*>>
) {
if (element !is SmkRuleOrCheckpoint) {
return
}
if (element is SmkUse) {
collectOverridingRulesOrCheckpoint(element, result)
}
collectOverriddenRuleOrCheckpoint(element, result)
}

private fun collectOverridingRulesOrCheckpoint(
use: SmkUse,
result: MutableCollection<in RelatedItemLineMarkerInfo<*>>
) {
val name = (use.nameIdentifier as? SmkUseNameIdentifier)?.getNameBeforeWildcard() ?: return
val inheritedRulesDeclaredExplicitly =
use.getDefinedReferencesOfImportedRuleNames()?.mapNotNull { it.reference.resolve() } ?: emptyList()
val ruleModule = use.getModuleName()?.reference?.resolve() as? SmkModule
val importedFile = ruleModule?.getPsiFile() as? SmkFile
val inheritedRulesDeclaredViaWildcard =
importedFile?.advancedCollectRules(mutableSetOf())?.map { it.second } ?: emptyList()
val results = inheritedRulesDeclaredExplicitly.ifEmpty { inheritedRulesDeclaredViaWildcard }
if (results.isEmpty()) {
return
}
val builder = NavigationGutterIconBuilder.create(AllIcons.Gutter.OverridingMethod)
.setTargets(results)
.setTooltipText(SnakemakeBundle.message("smk.line.marker.provider.overriding.rules"))
result.add(builder.createLineMarkerInfo(name))
}

private fun collectOverriddenRuleOrCheckpoint(
element: SmkRuleOrCheckpoint,
result: MutableCollection<in RelatedItemLineMarkerInfo<*>>
) {
val identifier = when (val identifier = element.nameIdentifier) {
is SmkUseNameIdentifier -> identifier.getNameBeforeWildcard()
else -> identifier
} ?: return
val module = element.let { ModuleUtilCore.findModuleForPsiElement(it.originalElement) } ?: return
val descendantsDefinedExplicitly = StubIndex.getElements(
SmkUseInheritedRulesIndex.KEY,
identifier.text,
module.project,
GlobalSearchScope.moduleWithDependentsScope(module),
SmkUse::class.java
)
val potentialDescendants = StubIndex.getElements(
SmkUseInheritedRulesIndex.KEY,
INHERITED_RULES_DECLARATION_VIA_WILDCARD,
module.project,
GlobalSearchScope.moduleWithDependentsScope(module),
SmkUse::class.java
)
val overrides = mutableListOf<SmkRuleOrCheckpoint>()
(descendantsDefinedExplicitly + potentialDescendants).forEach { descendant ->
// 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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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()

overrides.add(descendant)
}
}
if (overrides.isEmpty()) {
return
}
val builder = NavigationGutterIconBuilder.create(AllIcons.Gutter.OverridenMethod)
.setTargets(overrides)
.setTooltipText(SnakemakeBundle.message("smk.line.marker.provider.overridden.rules"))
result.add(builder.createLineMarkerInfo(identifier))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,6 @@ class SmkStatementParsing(

var lasTokenIsIdentifier =
!atToken(PyTokenTypes.IDENTIFIER) // Default value need to ve reversed
var simpleName = true // Does new rule name consist of one identifier
var hasIdentifier = false // Do we have new rule name
val name = myBuilder.mark()
while (true) {
Expand All @@ -538,13 +537,11 @@ class SmkStatementParsing(
}
lasTokenIsIdentifier = false
hasIdentifier = true
simpleName = false
nextToken()
}
PyTokenTypes.EXP, PyTokenTypes.COMMA -> {
myBuilder.error(SnakemakeBundle.message("PARSE.use.double.mult.sign"))
lasTokenIsIdentifier = false
simpleName = false
nextToken()
}
else -> break
Expand All @@ -556,11 +553,8 @@ class SmkStatementParsing(
// We can write 'as with:' or just 'as' and end line
// Both variants are allowed and original rule names will be taken
} else {
if (!simpleName) { // New rule name contains at least one '*' symbol
name.done(SmkElementTypes.USE_NAME_IDENTIFIER)
} else { // New rule name consists of one identifier
name.drop()
}
// Any SmkUse identifier is marked as USE_NAME_IDENTIFIER
name.done(SmkElementTypes.USE_NAME_IDENTIFIER)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ interface SmkUse : SmkRuleOrCheckpoint, StubBasedPsiElement<SmkUseStub> {
fun nameIdentifierIsWildcard(): Boolean
}

class SmkUseNameIdentifier(node: ASTNode) : PyElementImpl(node)
interface SmkUseNameIdentifier : PyElement {
fun isWildcard() : Boolean
fun getNameBeforeWildcard() : PsiElement
}
class SmkImportedRulesNames(node: ASTNode) : PyElementImpl(node)

interface SmkRuleOrCheckpointArgsSection : SmkArgsSection, PyTypedElement { // PyNamedElementContainer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package com.jetbrains.snakecharm.lang.psi.elementTypes
import com.intellij.psi.tree.TokenSet
import com.jetbrains.python.psi.PyElementType
import com.jetbrains.snakecharm.lang.psi.SmkImportedRulesNames
import com.jetbrains.snakecharm.lang.psi.SmkUseNameIdentifier
import com.jetbrains.snakecharm.lang.psi.impl.*

/**
Expand Down Expand Up @@ -38,7 +37,7 @@ object SmkElementTypes {
val USE_NAME_IDENTIFIER = PyElementType(
"SMK_USE_NAME_IDENTIFIER"
) {
SmkUseNameIdentifier(it)
SmkUseNameIdentifierImpl(it)
}

val USE_IMPORTED_RULES_NAMES = PyElementType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,9 @@ class SmkUseImpl : SmkRuleLikeImpl<SmkUseStub, SmkUse, SmkRuleOrCheckpointArgsSe
}

override fun getNameNode(): ASTNode? {
val identifier = super.getNameNode()
if (identifier != null) { // Returns new name if we know it
val namePattern = getUseNameIdentifier()
if (namePattern != null) { // Returns name identifier if it exits
// Example: use rule A as new_A
// Here we can detect name node by default
return identifier
}
val namePattern = getNameIdentifierPattern()
if (namePattern != null) { // Returns name patter if it exits
// Example: use rule A, B from M as new_*
// There are pattern instead of single node
return namePattern.node
Expand All @@ -60,18 +55,17 @@ class SmkUseImpl : SmkRuleLikeImpl<SmkUseStub, SmkUse, SmkRuleOrCheckpointArgsSe
}

override fun getProducedRulesNames(visitedFiles: MutableSet<PsiFile>): List<Pair<String, PsiElement>> {
val identifier = super.getNameNode()
if (identifier != null) {
return listOf(identifier.text to identifier.psi)
val newName = getUseNameIdentifier()
if (newName != null && !newName.text.contains('*')){
return listOf(newName.text to newName.originalElement)
}
val newName = getNameIdentifierPattern()
val originalNames = mutableListOf<Pair<String, PsiElement>>()
getDefinedReferencesOfImportedRuleNames()?.forEach { reference ->
originalNames.add(reference.text to reference)
}
if (originalNames.isEmpty()) {
val pairs = getPairsOfImportedRulesAndNames(visitedFiles) ?: return emptyList()
originalNames.addAll(pairs.map { it.first to it.second })
originalNames.addAll(pairs)
}
return if (newName != null) {
originalNames.map { newName.text.replace("*", it.first) to it.second }
Expand All @@ -81,9 +75,9 @@ class SmkUseImpl : SmkRuleLikeImpl<SmkUseStub, SmkUse, SmkRuleOrCheckpointArgsSe
}

/**
* Returns a [PsiElement] which sets pattern of produced rule names
* Returns a [SmkUseNameIdentifier] which may sets pattern of produced rule names or may be a simple name identifier
*/
private fun getNameIdentifierPattern() = findChildByType(SmkElementTypes.USE_NAME_IDENTIFIER) as? PsiElement
private fun getUseNameIdentifier() = findChildByType(SmkElementTypes.USE_NAME_IDENTIFIER) as? SmkUseNameIdentifier

/**
* Collects all imported rules and their names
Expand All @@ -101,15 +95,15 @@ class SmkUseImpl : SmkRuleLikeImpl<SmkUseStub, SmkUse, SmkRuleOrCheckpointArgsSe
override fun getModuleName() =
(findChildByType(SmkTokenTypes.SMK_FROM_KEYWORD) as? PsiElement)?.nextSibling?.nextSibling

override fun getDefinedReferencesOfImportedRuleNames(): Array<SmkReferenceExpression>? = PsiTreeUtil.getChildrenOfType(
findChildByType(USE_IMPORTED_RULES_NAMES),
SmkReferenceExpression::class.java
)
override fun getDefinedReferencesOfImportedRuleNames(): Array<SmkReferenceExpression>? =
PsiTreeUtil.getChildrenOfType(
findChildByType(USE_IMPORTED_RULES_NAMES),
SmkReferenceExpression::class.java
)

override fun getImportedRules(): List<SmkRuleOrCheckpoint>? =
getPairsOfImportedRulesAndNames(mutableSetOf())?.map { it.second }
getDefinedReferencesOfImportedRuleNames()?.mapNotNull { it.reference.resolve() as? SmkRuleOrCheckpoint }
?: getPairsOfImportedRulesAndNames(mutableSetOf())?.map { it.second }

override fun nameIdentifierIsWildcard() = nameIdentifier?.let {
it is SmkUseNameIdentifier && it.textContains('*')
} ?: false
override fun nameIdentifierIsWildcard() = (nameIdentifier as? SmkUseNameIdentifier)?.isWildcard() ?: false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.jetbrains.snakecharm.lang.psi.impl

import com.intellij.lang.ASTNode
import com.intellij.psi.PsiElement
import com.jetbrains.python.psi.impl.PyElementImpl
import com.jetbrains.snakecharm.lang.psi.SmkUseNameIdentifier

class SmkUseNameIdentifierImpl(node: ASTNode) : PyElementImpl(node), SmkUseNameIdentifier {
override fun isWildcard(): Boolean = text.contains('*')

override fun getNameBeforeWildcard(): PsiElement = firstChild
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,18 @@ class SmkRuleOrCheckpointNameReference(
return results
}
val allImportedFiles = smkFile.collectIncludedFiles()
val potentialModule = moduleRef?.reference?.resolve() as? SmkModule
return results.filter { resolveResult ->
// If we resolve module references, there must be only SmkModules
(resolveResult.element is SmkModule && itIsModuleMameReference) ||
// We don't want to suggest local resolve result for the reference of rule, which was imported
(moduleRef != null // Module name reference is defined and resolve result is from another file
&& element.containingFile.originalFile != resolveResult.element?.containingFile?.originalFile)
&& element.containingFile.originalFile != resolveResult.element?.containingFile?.originalFile
// Also, because of using indexes, we need to check if the resolve result file
// Connected to the file, which was declared in moduleRef, via 'include' or 'module'
// Later, allImportedFiles will be stored in cache
&& resolveResult.element?.containingFile?.originalFile in ((potentialModule?.getPsiFile() as? SmkFile)?.collectIncludedFiles()
?: listOf()))
// OR There are no 'from *name*' combination, so it hasn't been imported
|| (moduleRef == null && resolveResult.element?.containingFile?.originalFile in allImportedFiles)
}.toMutableList()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package com.jetbrains.snakecharm.lang.psi.impl.stubs

import com.intellij.psi.stubs.*
import com.jetbrains.python.psi.PyElement
import com.jetbrains.python.psi.PyStubElementType
import com.jetbrains.snakecharm.lang.psi.SmkRuleLike
import com.jetbrains.snakecharm.lang.psi.stubs.RuleDescendantStub
import com.jetbrains.snakecharm.lang.psi.stubs.SmkUseInheritedRulesIndex

abstract class SmkRuleDescendantElementType<StubT : RuleDescendantStub<*>, PsiT : PyElement>(
debugName: String,
private val nameIndexKey: StubIndexKey<String, out SmkRuleLike<*>>?
) :
PyStubElementType<StubT, PsiT>(debugName) {
abstract fun createStub(name: String?, inheritedRules: List<String?>, parentStub: StubElement<*>?): StubT

override fun serialize(stub: StubT, dataStream: StubOutputStream) {
dataStream.writeName(stub.name)
dataStream.writeInt(stub.getInheritedRulesNames().size)
stub.getInheritedRulesNames().forEach { name ->
dataStream.writeName(name)
}
}

override fun deserialize(dataStream: StubInputStream, parentStub: StubElement<*>?): StubT {
val name = dataStream.readNameString()
val size = dataStream.readInt()
val inheritedRules = mutableListOf<String?>()
for (x in 0 until size) {
inheritedRules.add(dataStream.readNameString())
}
return createStub(name, inheritedRules, parentStub)
}

override fun indexStub(stub: StubT, sink: IndexSink) {
if (nameIndexKey != null) {
stub.name?.let { name ->
sink.occurrence(nameIndexKey, name)
}
}
stub.getInheritedRulesNames().forEach { inheritedName ->
if (inheritedName != null) {
sink.occurrence(SmkUseInheritedRulesIndex.KEY, inheritedName)
}
}
super.indexStub(stub, sink)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ abstract class SmkRuleLikeStubImpl<StubT : NamedStub<PsiT>, PsiT>(
override fun getName() = myName
}

abstract class SmkRuleDescendantStubImpl<StubT : RuleDescendantStub<PsiT>, PsiT>(
private val myName: String?,
private val inheritedRulesNames: List<String?>,
parent: StubElement<*>?,
type: IStubElementType<StubT, PsiT>
) : StubBase<PsiT>(parent, type), RuleDescendantStub<PsiT> where PsiT : PsiNamedElement, PsiT : PyElement {
override fun getName() = myName
override fun getInheritedRulesNames() = inheritedRulesNames
}

class SmkCheckpointStubImpl(
name: String?,
parent: StubElement<*>?
Expand All @@ -42,5 +52,7 @@ class SmkModuleStubImpl(

class SmkUseStubImpl(
name: String?,
inheritedRulesNames: List<String?>,
parent: StubElement<*>?
) : SmkRuleLikeStubImpl<SmkUseStub, SmkUse>(name, parent, USE_DECLARATION_STATEMENT), SmkUseStub
) : SmkRuleDescendantStubImpl<SmkUseStub, SmkUse>(name, inheritedRulesNames, parent, USE_DECLARATION_STATEMENT),
SmkUseStub
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,25 @@ import com.intellij.psi.PsiElement
import com.intellij.psi.stubs.StubElement
import com.jetbrains.snakecharm.lang.psi.SmkUse
import com.jetbrains.snakecharm.lang.psi.impl.SmkUseImpl
import com.jetbrains.snakecharm.lang.psi.stubs.SmkUseInheritedRulesIndex.Companion.INHERITED_RULES_DECLARATION_VIA_WILDCARD
import com.jetbrains.snakecharm.lang.psi.stubs.SmkUseNameIndex.Companion.KEY
import com.jetbrains.snakecharm.lang.psi.stubs.SmkUseStub

class SmkUseElementType
: SmkRuleLikeElementType<SmkUseStub, SmkUse>("SMK_USE_DECLARATION_STATEMENT", KEY) {
: SmkRuleDescendantElementType<SmkUseStub, SmkUse>("SMK_USE_DECLARATION_STATEMENT", KEY) {

override fun createStub(name: String?, parentStub: StubElement<*>?): SmkUseStub =
SmkUseStubImpl(name, parentStub)
lateinit var psi: SmkUse
Copy link
Contributor

Choose a reason for hiding this comment

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

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.


override fun createStub(name: String?, inheritedRules: List<String?>, parentStub: StubElement<*>?) =
SmkUseStubImpl(name, inheritedRules, parentStub)

override fun createStub(psi: SmkUse, parentStub: StubElement<out PsiElement>?): SmkUseStub =
createStub(psi.name, parentStub)
createStub(
psi.name,
psi.getDefinedReferencesOfImportedRuleNames()?.map { it.text } ?: listOf(
Copy link
Contributor

Choose a reason for hiding this comment

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

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[]

Copy link
Contributor

Choose a reason for hiding this comment

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

SmkRuleDescendantStub.getInheritedRulesNames() seems should be also method of SmkImportedRulesNamesList.arguments stub.

INHERITED_RULES_DECLARATION_VIA_WILDCARD
),
parentStub).also { this.psi = psi }

override fun createPsi(stub: SmkUseStub): SmkUse = SmkUseImpl(stub)

Expand Down
Loading