From 582e2d2e08fb0a3e3a3270d5107fa8e8183e6188 Mon Sep 17 00:00:00 2001 From: Yoshitaka Jingu Date: Fri, 29 May 2026 14:37:05 +0900 Subject: [PATCH 1/4] feat: add Go to Declaration for Qiq helper calls Resolve `{{ helperName(...) }}` / `{{ $this->helperName(...) }}` in templates to the PHP class registered via HelperLocator::set(). Helper name -> class mapping is built by QiqHelperRegistry, which scans the bootstrap file(s) nominated in Settings > Languages & Frameworks > Qiq Templates (the closure's declared return type or `return new X(...)`). - QiqHelperGotoDeclarationHandler: navigation via gotoDeclarationHandler (function/method call names never aggregate contributed references, so a PsiReferenceContributor would be ignored). - QiqHelperInspectionSuppressor: suppress PhpUndefinedFunction/Method warnings for resolvable helper calls inside Qiq files (mirrors Blade). - Settings UI: file-chooser list for helper bootstrap files, stored relative to the project base dir when possible. Also fix a RAW_CONTENT injection bug: `{{= asset(...) }}` had its leading `a` stripped as the `{{a }}` escape modifier (corrupting the call to `sset(...)`). Helper names starting with h/a/j/u/c now pass through verbatim; the legacy text-modifier strip requires a standalone modifier byte followed by whitespace. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../helper/QiqHelperRegistry.kt | 257 ++++++++++++++++ .../idea_qiq_plugin/inject/QiqPhpInjector.kt | 32 +- .../QiqHelperInspectionSuppressor.kt | 52 ++++ .../lang/QiqInjectionSupport.kt | 33 +++ .../QiqHelperGotoDeclarationHandler.kt | 69 +++++ .../settings/QiqProjectConfigurable.kt | 106 ++++++- .../settings/QiqSettingService.kt | 43 ++- src/main/resources/META-INF/plugin.xml | 20 ++ .../resources/messages/QiqBundle.properties | 5 + .../helper/QiqHelperRegistryTest.kt | 280 ++++++++++++++++++ .../inject/QiqPhpInjectorTest.kt | 31 ++ 11 files changed, 907 insertions(+), 21 deletions(-) create mode 100644 src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistry.kt create mode 100644 src/main/kotlin/io/github/jingu/idea_qiq_plugin/inspection/QiqHelperInspectionSuppressor.kt create mode 100644 src/main/kotlin/io/github/jingu/idea_qiq_plugin/lang/QiqInjectionSupport.kt create mode 100644 src/main/kotlin/io/github/jingu/idea_qiq_plugin/navigation/QiqHelperGotoDeclarationHandler.kt create mode 100644 src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistryTest.kt diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistry.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistry.kt new file mode 100644 index 0000000..a6ec911 --- /dev/null +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistry.kt @@ -0,0 +1,257 @@ +package io.github.jingu.idea_qiq_plugin.helper + +import com.intellij.openapi.components.Service +import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.project.Project +import com.intellij.openapi.vfs.VirtualFile +import com.intellij.psi.PsiFile +import com.intellij.psi.PsiManager +import com.intellij.psi.util.PsiTreeUtil +import com.jetbrains.php.PhpIndex +import com.jetbrains.php.lang.psi.elements.ClassReference +import com.jetbrains.php.lang.psi.elements.Function +import com.jetbrains.php.lang.psi.elements.MethodReference +import com.jetbrains.php.lang.psi.elements.NewExpression +import com.jetbrains.php.lang.psi.elements.PhpClass +import com.jetbrains.php.lang.psi.elements.PhpReturn +import com.jetbrains.php.lang.psi.elements.StringLiteralExpression +import io.github.jingu.idea_qiq_plugin.settings.QiqSettingsService +import java.util.concurrent.ConcurrentHashMap + +/** + * Project-scoped registry that maps a Qiq helper name (the string used in + * templates such as `{{ myHelper(...) }}` / `{{ $this->myHelper(...) }}`) to + * the PHP class that the corresponding `HelperLocator::set()` factory + * returns. + * + * The mapping is built by statically inspecting one or more "bootstrap" + * files that the user nominates in Settings. Inside each file we scan for + * method calls of the form + * + * ```php + * $locator->set('name', static function () use (...): ClassName { + * return new ClassName(...); + * }); + * // or + * $locator->set('name', static fn (): ClassName => new ClassName(...)); + * ``` + * + * Resolution priority for the returned class is: + * 1. The closure's declared return type (`function (): ClassName { ... }`) + * 2. A `return new ClassName(...)` statement in the closure body + * 3. For arrow functions, the body expression if it is `new ClassName(...)` + * + * Any closure that does none of the above is ignored — those entries cannot + * be statically attributed and the user can supply an explicit override in + * the future if that turns out to be needed. + * + * Cache invalidation is driven by the modification stamp of every + * bootstrap file plus the configured set of bootstrap paths. + */ +@Service(Service.Level.PROJECT) +class QiqHelperRegistry(private val project: Project) { + + private data class CacheKey(val stamps: Map) + private data class CacheValue(val nameToFqn: Map) + + // ConcurrentHashMap because resolve() may be called from parallel + // ReadActions (multiple PsiReference resolutions across files). + private val cache = ConcurrentHashMap() + + /** + * Returns every helper name currently registered across all bootstrap + * files. Useful for completion or diagnostics; not exercised by the + * navigation path itself. + */ + fun allHelperNames(): Set = computeMap().keys + + /** Returns the FQN registered for [name], or null when unknown. */ + fun resolveFqn(name: String): String? = computeMap()[name] + + /** Resolves [name] to live [PhpClass] PSI elements via [PhpIndex]. */ + fun resolveClasses(name: String): Collection { + val fqn = resolveFqn(name) ?: return emptyList() + return PhpIndex.getInstance(project).getAnyByFQN(fqn) + } + + private fun computeMap(): Map { + val settings = QiqSettingsService.getInstance(project) + val configured = settings.getHelperBootstrapFiles() + if (configured.isEmpty()) { + if (log.isDebugEnabled) log.debug("Qiq helper registry: no bootstrap files configured") + return emptyMap() + } + + val files = configured.mapNotNull { settings.resolveHelperBootstrapFile(it) } + if (files.isEmpty()) { + if (log.isDebugEnabled) { + log.debug("Qiq helper registry: none of the configured paths resolved: $configured") + } + return emptyMap() + } + + val stamps = files.associate { it.path to it.modificationStamp } + val key = CacheKey(stamps) + cache[key]?.let { return it.nameToFqn } + + // Drop entries with different snapshots to keep the cache bounded + // even as users edit bootstrap files repeatedly. + cache.keys.removeIf { it != key } + + val merged = mutableMapOf() + val pm = PsiManager.getInstance(project) + for (vf in files) { + val psi = pm.findFile(vf) ?: continue + extractFromFile(psi, merged) + } + + if (log.isDebugEnabled) { + log.debug("Qiq helper registry: scanned ${files.size} file(s), ${merged.size} helper(s): ${merged.keys}") + } + + val value = CacheValue(merged.toMap()) + cache[key] = value + return value.nameToFqn + } + + /** + * Test-visible: extract `name → fqn` registrations from a single + * bootstrap file PSI without touching settings or the cache. Tests + * construct an in-memory PsiFile and invoke this directly so the + * scanner can be exercised without the LocalFileSystem dance. + */ + fun scanForTests(file: PsiFile): Map { + val sink = mutableMapOf() + extractFromFile(file, sink) + return sink + } + + private fun extractFromFile(file: PsiFile, sink: MutableMap) { + PsiTreeUtil.processElements(file) { element -> + if (element is MethodReference && element.name in REGISTRATION_METHOD_NAMES) { + handleRegistration(element, sink) + } + true + } + } + + private fun handleRegistration(ref: MethodReference, sink: MutableMap) { + val args = ref.parameters + val nameLiteral = args.getOrNull(0) as? StringLiteralExpression ?: return + val factory = factoryFunctionFromArg(args.getOrNull(1)) ?: return + + val name = nameLiteral.contents + if (name.isBlank()) return + + val fqn = extractFactoryReturnFqn(factory) ?: return + // Last write wins. Bootstrap files are typically authored with one + // canonical registration per name, so collisions are unexpected. + sink[name] = fqn + } + + /** + * PHP's PSI wraps anonymous closures in a generic expression node, so + * `args[1]` is typically a `PhpExpressionImpl` containing a [Function] + * child rather than a [Function] itself. Unwrap it here so both + * shapes succeed: + * + * ``` + * $x->set('a', function () { ... }) // arg = PhpExpression > Function + * $x->set('a', fn () => new X()) // same + * ``` + */ + private fun factoryFunctionFromArg(arg: Any?): Function? { + val element = arg as? com.intellij.psi.PsiElement ?: return null + if (element is Function) return element + // Look one level in; closures appear immediately under the wrapper. + return PsiTreeUtil.findChildOfType(element, Function::class.java) + } + + private fun extractFactoryReturnFqn(func: Function): String? { + // 1. Declared return type. Hpplus.Spur's QiqHelperLocatorProvider + // is annotated this way on every closure, so this path covers the + // common case without descending into the body. + declaredReturnFqn(func)?.let { return it } + + // 2. Walk the body for a `return new X(...)` statement (regular + // closures) or look at the body expression directly (arrow + // functions, which have no PhpReturn). + bodyNewExpressionFqn(func)?.let { return it } + + return null + } + + private fun declaredReturnFqn(func: Function): String? { + val declared = func.getLocalType(false) + val classFqns = declared.types.filter { isClassFqn(it) } + if (classFqns.size != 1) return null + return classFqns.first() + } + + private fun bodyNewExpressionFqn(func: Function): String? { + val returns = PsiTreeUtil.findChildrenOfType(func, PhpReturn::class.java) + .filter { PsiTreeUtil.getParentOfType(it, Function::class.java) === func } + for (ret in returns) { + val new = PsiTreeUtil.findChildOfType(ret, NewExpression::class.java) ?: continue + classRefFqn(new.classReference)?.let { return it } + } + + // Arrow functions: no PhpReturn — the body expression itself is the + // value. Search shallowly within the function for a NewExpression + // whose nearest enclosing Function is this arrow function. + val arrowNews = PsiTreeUtil.findChildrenOfType(func, NewExpression::class.java) + .filter { PsiTreeUtil.getParentOfType(it, Function::class.java) === func } + for (new in arrowNews) { + classRefFqn(new.classReference)?.let { return it } + } + return null + } + + private fun classRefFqn(ref: ClassReference?): String? { + if (ref == null) return null + val fqn = ref.fqn + return if (!fqn.isNullOrBlank()) fqn else null + } + + private fun isClassFqn(type: String): Boolean { + // Class FQNs in PhpType start with `\` followed by an identifier + // segment. Built-in scalar / pseudo types use the same prefix but + // are excluded explicitly to avoid e.g. `\void` slipping through. + if (!type.startsWith("\\")) return false + if (type in BUILTIN_PSEUDO_TYPES) return false + // Avoid generics / `?nullable` / intersection / array shapes. + if (type.any { it == '|' || it == '&' || it == '?' || it == '<' || it == '(' }) return false + return true + } + + /** + * Forget every cached entry. Called by [QiqProjectConfigurable] when + * the bootstrap-file list changes (the modification stamps stay the + * same so the natural snapshot-keyed invalidation does not fire), and + * by tests that mutate fixtures between assertions. + */ + fun invalidateCache() { + cache.clear() + } + + companion object { + private val log = Logger.getInstance(QiqHelperRegistry::class.java) + + fun getInstance(project: Project): QiqHelperRegistry = + project.getService(QiqHelperRegistry::class.java) + + // HelperLocator's public API in Qiq 1.x is `set(name, factory)`. + // `setFactory` and `register` are accepted as common subclass + // additions (e.g. BEAR\QiqModule\HelperLocator only uses `set`, + // but other community wrappers occasionally expose either). + private val REGISTRATION_METHOD_NAMES = setOf("set", "setFactory", "register") + + private val BUILTIN_PSEUDO_TYPES = setOf( + "\\void", "\\mixed", "\\never", "\\null", "\\true", "\\false", + "\\int", "\\integer", "\\bool", "\\boolean", "\\string", + "\\float", "\\double", "\\array", "\\iterable", "\\object", + "\\callable", "\\callback", "\\resource", "\\number", + "\\class-string", "\\static", "\\self", "\\parent", "\\this" + ) + } +} diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inject/QiqPhpInjector.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inject/QiqPhpInjector.kt index 9c99e06..79323f9 100644 --- a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inject/QiqPhpInjector.kt +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inject/QiqPhpInjector.kt @@ -174,9 +174,22 @@ class QiqPhpInjector : MultiHostInjector, DumbAware { prefix = "" } - firstChar in ESCAPE_MODIFIER_CHARS -> { - // Legacy / text-fallback: strip the modifier byte before - // routing through QiqRuntimeFunctions*. + host.node.elementType == QiqTokenTypes.RAW_CONTENT -> { + // {{= ... }}: the lexer consumed the `=` opener, so the + // content is already the expression. Emit a plain echo + // without touching the leading byte — critical for helper + // names that happen to start with an escape-modifier + // letter (asset/currentUrl/upper → a/c/u), which must NOT + // be mistaken for the {{a }}/{{c }}/{{u }} directives. + prefix = "" + } + isStandaloneEscapeModifier(trimmedContent) -> { + // Legacy / text-fallback: a single escape-modifier byte + // followed by whitespace (e.g. "h $x"). Strip it and route + // through QiqRuntimeFunctions*. The whitespace requirement + // prevents `asset(...)` ('a' + 's') from being treated as + // the `{{a }}` directive. val strippedStart = skipLeadingWhitespace(raw, range.startOffset + 1, range.endOffset) ?: return null injectionRange = TextRange(strippedStart, range.endOffset) @@ -185,7 +198,8 @@ class QiqPhpInjector : MultiHostInjector, DumbAware { suffix = ") ?>" } firstChar == '=' -> { - // {{= ... }}: raw echo, strip the `=` and emit a plain echo tag. + // Legacy {{= ... }} shape where the `=` is still in the + // content: strip it and emit a plain echo tag. val strippedStart = skipLeadingWhitespace(raw, range.startOffset + 1, range.endOffset) ?: return null injectionRange = TextRange(strippedStart, range.endOffset) @@ -237,6 +251,16 @@ class QiqPhpInjector : MultiHostInjector, DumbAware { else -> null } + /** + * True when [content] begins with a genuine standalone escape modifier: + * a single `h`/`a`/`j`/`u`/`c` byte immediately followed by whitespace + * (e.g. `h $x`). This distinguishes the legacy text form of an escape + * directive from a helper/function call whose name merely starts with one + * of those letters (e.g. `asset(...)`, `currentUrl(...)`, `upper(...)`). + */ + private fun isStandaloneEscapeModifier(content: String): Boolean = + content.length >= 2 && content[0] in ESCAPE_MODIFIER_CHARS && content[1].isWhitespace() + private fun skipLeadingWhitespace(raw: String, from: Int, until: Int): Int? { var i = from while (i < until && raw[i].isWhitespace()) i++ diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inspection/QiqHelperInspectionSuppressor.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inspection/QiqHelperInspectionSuppressor.kt new file mode 100644 index 0000000..85b2c8d --- /dev/null +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inspection/QiqHelperInspectionSuppressor.kt @@ -0,0 +1,52 @@ +package io.github.jingu.idea_qiq_plugin.inspection + +import com.intellij.codeInspection.InspectionSuppressor +import com.intellij.codeInspection.SuppressQuickFix +import com.intellij.psi.PsiElement +import com.intellij.psi.util.PsiTreeUtil +import com.jetbrains.php.lang.psi.elements.FunctionReference +import io.github.jingu.idea_qiq_plugin.helper.QiqHelperRegistry +import io.github.jingu.idea_qiq_plugin.lang.QiqInjectionSupport + +/** + * Suppresses PhpStorm's "undefined function/method" warnings for Qiq helper + * calls that the plugin can resolve via [QiqHelperRegistry]. + * + * A bare `{{ myHelper(...) }}` compiles to `$this->myHelper(...)`, which Qiq + * dispatches through `HelperLocator` at runtime. The injected PHP therefore + * references a function/method that has no static declaration, so + * PhpUndefinedFunctionInspection / PhpUndefinedMethodInspection flag it even + * though [QiqHelperGotoDeclarationHandler] can navigate to the helper class. + * This mirrors how the bundled Blade support suppresses PHP inspections + * inside Blade-injected fragments. + * + * Only names registered in a configured bootstrap file are suppressed; truly + * undefined calls keep their warning. + */ +class QiqHelperInspectionSuppressor : InspectionSuppressor { + + override fun isSuppressedFor(element: PsiElement, toolId: String): Boolean { + if (toolId !in SUPPRESSED_TOOLS) return false + + // The inspection registers the problem on the call (or its name leaf); + // accept either shape. MethodReference also implements FunctionReference. + val call = element as? FunctionReference + ?: PsiTreeUtil.getParentOfType(element, FunctionReference::class.java, false) + ?: return false + + if (!QiqInjectionSupport.isInQiqFile(call)) return false + + val name = call.name?.takeIf { it.isNotEmpty() } ?: return false + return QiqHelperRegistry.getInstance(element.project).resolveFqn(name) != null + } + + override fun getSuppressActions(element: PsiElement?, toolId: String): Array = + SuppressQuickFix.EMPTY_ARRAY + + private companion object { + private val SUPPRESSED_TOOLS = setOf( + "PhpUndefinedFunctionInspection", + "PhpUndefinedMethodInspection", + ) + } +} diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/lang/QiqInjectionSupport.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/lang/QiqInjectionSupport.kt new file mode 100644 index 0000000..a1802bc --- /dev/null +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/lang/QiqInjectionSupport.kt @@ -0,0 +1,33 @@ +package io.github.jingu.idea_qiq_plugin.lang + +import com.intellij.lang.injection.InjectedLanguageManager +import com.intellij.psi.PsiElement +import com.intellij.psi.templateLanguages.TemplateLanguageFileViewProvider + +/** + * Shared detection for "is this element part of a Qiq template (possibly via + * injected PHP)". Used by helper navigation and inspection suppression so the + * detection rules stay in one place. + */ +object QiqInjectionSupport { + + fun isInQiqFile(element: PsiElement): Boolean { + val project = element.project + val ilm = InjectedLanguageManager.getInstance(project) + val topLevel = ilm.getTopLevelFile(element) ?: element.containingFile ?: return false + + val viewProvider = topLevel.viewProvider + if (viewProvider is TemplateLanguageFileViewProvider && viewProvider.baseLanguage == QiqTemplateLanguage) { + return true + } + if (topLevel.language == QiqTemplateLanguage) return true + + val vf = topLevel.virtualFile ?: return false + if (vf.fileType == QiqFileType) return true + + val name = vf.name + if (name.endsWith(".qiq") || name.endsWith(".qiq.php")) return true + + return vf.getUserData(QiqFileTypeOverrider.QIQ_MARKER) == true + } +} diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/navigation/QiqHelperGotoDeclarationHandler.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/navigation/QiqHelperGotoDeclarationHandler.kt new file mode 100644 index 0000000..835ee29 --- /dev/null +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/navigation/QiqHelperGotoDeclarationHandler.kt @@ -0,0 +1,69 @@ +package io.github.jingu.idea_qiq_plugin.navigation + +import com.intellij.codeInsight.navigation.actions.GotoDeclarationHandler +import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.editor.Editor +import com.intellij.psi.PsiElement +import com.intellij.psi.util.PsiTreeUtil +import com.jetbrains.php.lang.psi.elements.FunctionReference +import com.jetbrains.php.lang.psi.elements.PhpClass +import io.github.jingu.idea_qiq_plugin.helper.QiqHelperRegistry +import io.github.jingu.idea_qiq_plugin.lang.QiqInjectionSupport + +/** + * Go to Declaration for Qiq helper calls inside templates. + * + * A bare `{{ myHelper(...) }}` is compiled by Qiq into `$this->myHelper(...)` + * and surfaces in the injected PHP as a [FunctionReference] (bare call) or a + * MethodReference (`$this->myHelper(...)`). Both implement + * `FunctionReference`. Because those elements carry their own (failing) + * resolution for an unregistered helper name, a `PsiReferenceContributor` + * is never consulted for them — only string literals aggregate contributed + * references. The canonical mechanism for adding navigation to call names is + * this [GotoDeclarationHandler]. + * + * Targets are sourced from [QiqHelperRegistry], which scans the bootstrap + * files configured in Settings > Languages & Frameworks > Qiq Templates. + */ +class QiqHelperGotoDeclarationHandler : GotoDeclarationHandler { + + override fun getGotoDeclarationTargets( + sourceElement: PsiElement?, + offset: Int, + editor: Editor?, + ): Array? { + val src = sourceElement ?: return null + + // Climb to the enclosing call. MethodReference also implements + // FunctionReference, so this catches both `helper(...)` and + // `$this->helper(...)`. + val call = PsiTreeUtil.getParentOfType(src, FunctionReference::class.java, false) ?: return null + + // Only fire when the caret sits on the call *name*, not on an + // argument. textRange comparison stays within a single PSI tree + // (the injected file), avoiding host/injected offset confusion. + val nameNode = call.nameNode ?: return null + if (!nameNode.textRange.contains(src.textRange)) return null + + if (!QiqInjectionSupport.isInQiqFile(call)) return null + + val name = call.name?.takeIf { it.isNotEmpty() } ?: return null + val classes = QiqHelperRegistry.getInstance(call.project).resolveClasses(name) + if (log.isDebugEnabled) { + log.debug("Qiq helper goto: name='$name' resolvedClasses=${classes.size}") + } + if (classes.isEmpty()) return null + + // Prefer the __invoke method — the actual call target Qiq dispatches + // to — falling back to the class declaration when none is declared. + val targets = classes.map { phpClass -> invokeMethodOf(phpClass) ?: phpClass } + return targets.toTypedArray() + } + + private fun invokeMethodOf(phpClass: PhpClass): PsiElement? = + phpClass.findMethodByName("__invoke") + + private companion object { + private val log = Logger.getInstance(QiqHelperGotoDeclarationHandler::class.java) + } +} diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqProjectConfigurable.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqProjectConfigurable.kt index 77370fc..fbff7af 100644 --- a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqProjectConfigurable.kt +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqProjectConfigurable.kt @@ -1,20 +1,36 @@ package io.github.jingu.idea_qiq_plugin.settings +import com.intellij.openapi.fileChooser.FileChooser +import com.intellij.openapi.fileChooser.FileChooserDescriptorFactory import com.intellij.openapi.options.BoundSearchableConfigurable import com.intellij.openapi.project.Project import com.intellij.openapi.ui.DialogPanel +import com.intellij.openapi.vfs.VirtualFile +import com.intellij.ui.CollectionListModel +import com.intellij.ui.ToolbarDecorator +import com.intellij.ui.components.JBList +import com.intellij.ui.dsl.builder.AlignX import com.intellij.ui.dsl.builder.bindSelected import com.intellij.ui.dsl.builder.panel +import io.github.jingu.idea_qiq_plugin.helper.QiqHelperRegistry import io.github.jingu.idea_qiq_plugin.ui.QiqBundle +import javax.swing.ListSelectionModel /** * Project-level settings page for Qiq Templates, surfaced under * Settings > Languages & Frameworks > Qiq Templates. * - * Currently exposes a single switch: Strict Types. Enabling it makes - * QiqPhpInjector prepend `` to each - * Qiq template's injected PHP, so scalar literal misuses such as - * `{{h true }}` or `{{h 123 }}` surface as PhpStorm type warnings. + * Exposes: + * - Strict Types switch (injects `declare(strict_types=1)` into Qiq + * template PHP fragments so scalar literal misuses surface as type + * warnings). + * - Helper bootstrap files list. Each entry points to a PHP file that + * registers Qiq helpers via `HelperLocator::set('name', closure)`. + * Files are added via a native file chooser (multi-select) and stored + * relative to the project base dir when possible so settings stay + * portable across checkouts. The scanner inspects these files to build + * the helperName → PhpClass map used for Go to Declaration on helper + * calls in templates. */ class QiqProjectConfigurable(private val project: Project) : BoundSearchableConfigurable( QiqBundle.message("settings.qiq.display.name"), @@ -23,19 +39,77 @@ class QiqProjectConfigurable(private val project: Project) : BoundSearchableConf ) { private val settings get() = QiqSettingsService.getInstance(project) + private val bootstrapModel = CollectionListModel() - override fun createPanel(): DialogPanel = panel { - row { - // Bind through the public service accessors rather than the - // backing `state` property, which is private. Using accessors - // also keeps the configurable independent of the persistent - // state's storage shape. - checkBox(QiqBundle.message("settings.qiq.strict.types.checkbox")) - .bindSelected( - { settings.isStrictTypesEnabled() }, - { settings.setStrictTypesEnabled(it) }, - ) - .comment(QiqBundle.message("settings.qiq.strict.types.comment")) + override fun createPanel(): DialogPanel { + val list = JBList(bootstrapModel).apply { + selectionMode = ListSelectionModel.SINGLE_SELECTION + visibleRowCount = 4 + emptyText.text = QiqBundle.message("settings.qiq.helper.bootstrap.empty") } + val listPanel = ToolbarDecorator.createDecorator(list) + .setAddAction { chooseBootstrapFiles() } + .disableUpDownActions() + .createPanel() + + return panel { + row { + // Bind through the public service accessors rather than the + // backing `state` property, which is private. + checkBox(QiqBundle.message("settings.qiq.strict.types.checkbox")) + .bindSelected( + { settings.isStrictTypesEnabled() }, + { settings.setStrictTypesEnabled(it) }, + ) + .comment(QiqBundle.message("settings.qiq.strict.types.comment")) + } + + group(QiqBundle.message("settings.qiq.helper.bootstrap.group")) { + row { + cell(listPanel) + .align(AlignX.FILL) + .resizableColumn() + .comment(QiqBundle.message("settings.qiq.helper.bootstrap.comment")) + .onReset { bootstrapModel.replaceAll(settings.getHelperBootstrapFiles()) } + .onIsModified { bootstrapModel.items != settings.getHelperBootstrapFiles() } + .onApply { + settings.setHelperBootstrapFiles(bootstrapModel.items.toList()) + // Drop cached scan results so the next resolve + // sees the new file list immediately. + QiqHelperRegistry.getInstance(project).invalidateCache() + } + }.resizableRow() + } + } + } + + private fun chooseBootstrapFiles() { + val descriptor = FileChooserDescriptorFactory.createMultipleFilesNoJarsDescriptor() + .withTitle(QiqBundle.message("settings.qiq.helper.bootstrap.chooser.title")) + .withFileFilter { it.extension.equals("php", ignoreCase = true) } + + val toSelect = project.basePath?.let { com.intellij.openapi.vfs.LocalFileSystem.getInstance().findFileByPath(it) } + FileChooser.chooseFiles(descriptor, project, toSelect) { files -> + files.forEach { vf -> + val stored = toStoredPath(vf) + if (bootstrapModel.items.none { it == stored }) { + bootstrapModel.add(stored) + } + } + } + } + + /** + * Store paths relative to the project base dir when the chosen file + * lives under it; fall back to the absolute path otherwise. This keeps + * settings portable when the repository is checked out elsewhere. + */ + private fun toStoredPath(vf: VirtualFile): String { + val base = project.basePath + val path = vf.path + if (base != null && path.startsWith("$base/")) { + return path.removePrefix("$base/") + } + return path } } diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqSettingService.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqSettingService.kt index cacb2fa..d42a4ad 100644 --- a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqSettingService.kt +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqSettingService.kt @@ -32,7 +32,14 @@ class QiqSettingsService(private val project: Project) : PersistentStateComponen // escape directives (e.g. `{{h true }}`, `{{h 123 }}`) surface as // PhpStorm type warnings. Off by default to match Qiq's runtime // behaviour, which performs implicit scalar→string casts. - var enableStrictTypes: Boolean = false + var enableStrictTypes: Boolean = false, + // Paths to PHP files that register Qiq helpers via + // `$locator->set('name', closure)` on Qiq\HelperLocator (or a + // subclass such as BEAR\QiqModule\HelperLocator). QiqHelperRegistry + // scans these to build a helperName → PHP class map for Go to + // Declaration in templates. Paths are interpreted relative to the + // project content root when not absolute. + var helperBootstrapFiles: MutableList = mutableListOf() ) private var state = State() @@ -52,6 +59,40 @@ class QiqSettingsService(private val project: Project) : PersistentStateComponen state.enableStrictTypes = enabled } + /** Bootstrap files registered for HelperLocator scanning. Returned as a defensive copy. */ + fun getHelperBootstrapFiles(): List = state.helperBootstrapFiles.toList() + + fun setHelperBootstrapFiles(paths: List) { + state.helperBootstrapFiles = paths.toMutableList() + } + + /** + * Resolve a configured helper bootstrap path (absolute, or relative to + * the project base dir / a content root) to a VirtualFile. Returns null + * when the file does not exist or is not a regular file. + */ + fun resolveHelperBootstrapFile(raw: String): VirtualFile? { + val path = raw.trim() + if (path.isEmpty()) return null + val lfs = LocalFileSystem.getInstance() + + if (File(path).isAbsolute) { + return lfs.findFileByPath(path)?.takeIf { it.isValid && !it.isDirectory } + } + + val relative = path.removePrefix("/") + val roots = buildList { + project.basePath?.let(lfs::findFileByPath)?.let(::add) + addAll(com.intellij.openapi.roots.ProjectRootManager.getInstance(project).contentRoots) + } + for (root in roots) { + root.findFileByRelativePath(relative) + ?.takeIf { it.isValid && !it.isDirectory } + ?.let { return it } + } + return null + } + /** * Entry point for contributors (completion/navigation/etc.). diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index b8cc784..7736d9e 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -56,6 +56,26 @@ + + + + + + set('name', closure). The closure's declared return type or return new ClassName(...) supplies the navigation target for {{ helperName(...) }} / {{ $this->helperName(...) }} in templates. +settings.qiq.helper.bootstrap.empty=No bootstrap files. Click + to choose a PHP file that calls $locator->set(...). +settings.qiq.helper.bootstrap.chooser.title=Select Qiq Helper Bootstrap Files diff --git a/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistryTest.kt b/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistryTest.kt new file mode 100644 index 0000000..a03900b --- /dev/null +++ b/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistryTest.kt @@ -0,0 +1,280 @@ +package io.github.jingu.idea_qiq_plugin.helper + +import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiFile +import com.intellij.psi.PsiFileFactory +import com.intellij.testFramework.junit5.RunInEdt +import com.intellij.testFramework.junit5.impl.TestApplicationExtension +import com.intellij.testFramework.junit5.resources.ProjectExtension +import com.jetbrains.php.lang.PhpFileType +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import org.junit.jupiter.api.extension.RegisterExtension +import kotlin.test.assertEquals + +/** + * Unit-tests the static scanner that powers Go to Declaration for Qiq + * helper calls. The shapes covered here mirror the + * `QiqHelperLocatorProvider` style used in real-world projects: + * + * - `static function () use (...): ClassName { return new ClassName(...); }` + * - `static fn (): ClassName => new ClassName(...)` + * - `function () { return new ClassName(...); }` (no declared return) + * - `fn () => new ClassName(...)` (no declared return) + * - `$x->register(...)` and `$x->setFactory(...)` aliases + * + * Combined, these patterns cover every `$locator->set(...)` call observed + * in Hpplus.Spur's `QiqHelperLocatorProvider`. + */ +@RunInEdt +@ExtendWith(TestApplicationExtension::class) +class QiqHelperRegistryTest { + + companion object { + @JvmField + @RegisterExtension + val projectExtension = ProjectExtension() + } + + @Test + fun extractsDeclaredReturnTypeFromClosure(project: Project) { + val map = scan( + project, + """ + set('anchor', static function () use (${'$'}locator): Anchor { + return new Anchor(${'$'}locator->get('escape')); + }); + } + } + """.trimIndent(), + ) + assertEquals("\\Qiq\\Helper\\Anchor", map["anchor"]) + } + + @Test + fun extractsDeclaredReturnTypeFromArrowFunction(project: Project) { + val map = scan( + project, + """ + set('getLoginUrl', static fn (): GetLoginUrl => new GetLoginUrl()); + } + } + """.trimIndent(), + ) + assertEquals("\\Qiq\\Helper\\GetLoginUrl", map["getLoginUrl"]) + } + + @Test + fun fallsBackToReturnNewExpressionWhenReturnTypeIsMissing(project: Project) { + val map = scan( + project, + """ + set('textField', static function () use (${'$'}locator) { + return new TextField(${'$'}locator->get('escape')); + }); + } + } + """.trimIndent(), + ) + assertEquals("\\Qiq\\Helper\\TextField", map["textField"]) + } + + @Test + fun fallsBackToArrowBodyNewExpressionWhenReturnTypeIsMissing(project: Project) { + val map = scan( + project, + """ + set('image', static fn () => new Image()); + } + } + """.trimIndent(), + ) + assertEquals("\\Qiq\\Helper\\Image", map["image"]) + } + + @Test + fun acceptsRegisterAndSetFactoryAliases(project: Project) { + val map = scan( + project, + """ + register('foo', static fn (): Foo => new Foo()); + ${'$'}locator->setFactory('bar', static fn (): Bar => new Bar()); + } + } + """.trimIndent(), + ) + assertEquals("\\Qiq\\Helper\\Foo", map["foo"]) + assertEquals("\\Qiq\\Helper\\Bar", map["bar"]) + } + + @Test + fun ignoresRegistrationWithNonStringName(project: Project) { + val map = scan( + project, + """ + set(${'$'}name, static fn (): Foo => new Foo()); + } + } + """.trimIndent(), + ) + assertEquals(0, map.size, "Dynamic registrations must not be indexed, got: $map") + } + + @Test + fun ignoresRegistrationWithUnresolvableFactory(project: Project) { + val map = scan( + project, + """ + set('proxy', static function () use (${'$'}svc) { + return ${'$'}svc->make('thing'); + }); + } + } + """.trimIndent(), + ) + assertEquals(0, map.size, "Indirect factories must not be indexed, got: $map") + } + + @Test + fun extractsMultipleRegistrationsFromProviderStyleFile(project: Project) { + val map = scan( + project, + providerStyleFixture(), + ) + assertEquals("\\Qiq\\Helper\\CardIcon", map["getCardIcon"]) + assertEquals("\\Qiq\\Helper\\ImageFlux", map["imageFlux"]) + assertEquals("\\Qiq\\Helper\\EscapeJs", map["j"]) + assertEquals("\\Qiq\\Helper\\GetLoginUrl", map["getLoginUrl"]) + assertEquals("\\Qiq\\Helper\\GetSignupUrl", map["getSignupUrl"]) + assertEquals(5, map.size, "Unexpected entries: $map") + } + + // ----------------------------------------------------------------- + + private fun scan(project: Project, php: String): Map { + val factory = PsiFileFactory.getInstance(project) + var psi: PsiFile? = null + ApplicationManager.getApplication().runReadAction { + psi = factory.createFileFromText("Bootstrap.php", PhpFileType.INSTANCE, php) + } + val file = psi ?: error("Failed to create PHP PsiFile") + var result: Map = emptyMap() + ApplicationManager.getApplication().runReadAction { + result = QiqHelperRegistry.getInstance(project).scanForTests(file) + } + return result + } + + private fun providerStyleFixture(): String = """ + set('getCardIcon', static function () use (${'$'}locator): CardIcon { + return new CardIcon(${'$'}locator->get('escape')); + }); + + ${'$'}locator->set('imageFlux', static function () use (${'$'}locator): ImageFlux { + return new ImageFlux(${'$'}locator->get('escape')); + }); + + ${'$'}locator->set('j', static function () use (${'$'}locator): EscapeJs { + return new EscapeJs(${'$'}locator->get('escape')); + }); + + ${'$'}escape = ${'$'}locator->get('escape'); + ${'$'}locator->set('getLoginUrl', static fn (): GetLoginUrl => new GetLoginUrl(${'$'}escape)); + ${'$'}locator->set('getSignupUrl', static fn (): GetSignupUrl => new GetSignupUrl(${'$'}escape)); + + return ${'$'}locator; + } + } + """.trimIndent() +} diff --git a/src/test/kotlin/io/github/jingu/idea_qiq_plugin/inject/QiqPhpInjectorTest.kt b/src/test/kotlin/io/github/jingu/idea_qiq_plugin/inject/QiqPhpInjectorTest.kt index 8bfab3b..6003e22 100644 --- a/src/test/kotlin/io/github/jingu/idea_qiq_plugin/inject/QiqPhpInjectorTest.kt +++ b/src/test/kotlin/io/github/jingu/idea_qiq_plugin/inject/QiqPhpInjectorTest.kt @@ -230,6 +230,37 @@ class QiqPhpInjectorTest { } } + @Test + fun rawEchoHelperCallStartingWithEscapeLetterIsNotStripped(project: Project) { + // Regression: `{{= asset(...) }}` is a RAW_CONTENT host whose content + // starts with `a`. The injector must NOT treat the leading `a` as the + // `{{a }}` attribute-escape modifier — doing so corrupted the call to + // `sset(...)`. Helper names beginning with h/a/j/u/c (asset, + // currentUrl, upper, ...) must pass through verbatim. + val cases = listOf( + "{{= asset('/js/common.js') }}" to "asset('/js/common.js')", + "{{= currentUrl(\$url) }}" to "currentUrl(\$url)", + "{{= upper(\$name) }}" to "upper(\$name)", + ) + + ApplicationManager.getApplication().runReadAction { + for ((template, expectedExpr) in cases) { + val psiFile = createQiqFile(project, template) + val host = PsiTreeUtil.collectElementsOfType(psiFile, QiqCodeHost::class.java).single() + + val registrar = RecordingRegistrar() + injector.getLanguagesToInject(registrar, host) + + val call = registrar.calls.single { it.language == PhpLanguage.INSTANCE } + assertEquals("", call.suffix, "Wrong suffix for $template") + + val injectedExpr = call.host.text.substring(call.range.startOffset, call.range.endOffset) + assertEquals(expectedExpr, injectedExpr, "Leading byte must not be stripped for $template") + } + } + } + private fun createQiqFile(project: Project, text: String): PsiFile { // In-memory PSI construction: no write lock required, and wrapping in // WriteAction throws if the caller is already inside a ReadAction (which From 7b9de281a40801b162df4d0731e964615f8e549d Mon Sep 17 00:00:00 2001 From: Yoshitaka Jingu Date: Fri, 29 May 2026 14:51:14 +0900 Subject: [PATCH 2/4] feat: auto-discover Qiq 2.x/3.x Helpers subclass methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add navigation + warning suppression for the Qiq 2.x/3.x official custom helper style: public methods on a subclass of Qiq\Helpers (typically extending Qiq\Helper\Html\HtmlHelpers), passed via Template::new(helpers:). QiqHelpersClassResolver walks Qiq\Helpers subclasses via PhpIndex and indexes their public, non-static, non-magic own methods (helperName -> Method), cached against PsiModificationTracker. Library classes under the \Qiq\ namespace are skipped — their built-ins resolve through the qiq_runtime.php stub. No user configuration is required; it self-gates (a 1.x project has no \Qiq\Helpers, so the walk is empty). QiqHelperGotoDeclarationHandler now offers both resolution paths (1.x HelperLocator classes and 2.x/3.x Helpers methods); the inspection suppressor likewise suppresses undefined-function/method warnings for either. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../helper/QiqHelpersClassResolver.kt | 81 +++++++++++++++++++ .../QiqHelperInspectionSuppressor.kt | 7 +- .../QiqHelperGotoDeclarationHandler.kt | 17 +++- 3 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolver.kt diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolver.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolver.kt new file mode 100644 index 0000000..c7396fb --- /dev/null +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolver.kt @@ -0,0 +1,81 @@ +package io.github.jingu.idea_qiq_plugin.helper + +import com.intellij.openapi.components.Service +import com.intellij.openapi.project.Project +import com.intellij.psi.util.CachedValue +import com.intellij.psi.util.CachedValueProvider +import com.intellij.psi.util.CachedValuesManager +import com.intellij.psi.util.PsiModificationTracker +import com.jetbrains.php.PhpIndex +import com.jetbrains.php.lang.psi.elements.Method +import com.jetbrains.php.lang.psi.elements.PhpModifier + +/** + * Resolves Qiq 2.x / 3.x custom helpers. + * + * The official docs define a custom helper as a public method on a subclass + * of `Qiq\Helpers` (typically extending `Qiq\Helper\Html\HtmlHelpers`), + * passed via `Template::new(helpers: new CustomHelpers())`. Templates then + * call it as `{{ name(...) }}` / `{{ $this->name(...) }}`. + * + * Unlike the 1.x HelperLocator style ([QiqHelperRegistry], which needs the + * user to nominate bootstrap files), this resolver is fully automatic: every + * project-defined `Qiq\Helpers` subclass is discovered through [PhpIndex]. + * Library classes under the `\Qiq\` namespace (Helpers / HtmlHelpers and the + * built-in helper methods) are skipped — those built-ins already resolve via + * the bundled qiq_runtime.php stub. + * + * Self-gating: in a 1.x project `\Qiq\Helpers` does not exist, so the + * subclass walk yields nothing and this resolver is a no-op. + */ +@Service(Service.Level.PROJECT) +class QiqHelpersClassResolver(private val project: Project) { + + // helperName -> public method declarations across user-defined + // Qiq\Helpers subclasses. Rebuilt whenever PSI changes. + private val methodIndex: CachedValue>> = + CachedValuesManager.getManager(project).createCachedValue { + CachedValueProvider.Result.create( + buildIndex(), + PsiModificationTracker.getInstance(project), + ) + } + + /** Public helper methods declared for [name], or empty when unknown. */ + fun resolve(name: String): List = methodIndex.value[name].orEmpty() + + fun hasHelper(name: String): Boolean = methodIndex.value.containsKey(name) + + private fun buildIndex(): Map> { + val index = PhpIndex.getInstance(project) + val result = mutableMapOf>() + index.processAllSubclasses(HELPERS_BASE_FQN) { phpClass -> + // Only user-defined subclasses contribute; library helper classes + // (\Qiq\Helpers, \Qiq\Helper\Html\HtmlHelpers, ...) are covered by + // the runtime stub already. + if (!phpClass.fqn.startsWith(QIQ_NAMESPACE_PREFIX)) { + for (method in phpClass.ownMethods) { + if (isHelperMethod(method)) { + result.getOrPut(method.name) { mutableListOf() }.add(method) + } + } + } + true + } + return result + } + + private fun isHelperMethod(method: Method): Boolean = + method.access == PhpModifier.Access.PUBLIC && + !method.isStatic && + !method.isAbstract && + !method.name.startsWith("__") + + companion object { + private const val HELPERS_BASE_FQN = "\\Qiq\\Helpers" + private const val QIQ_NAMESPACE_PREFIX = "\\Qiq\\" + + fun getInstance(project: Project): QiqHelpersClassResolver = + project.getService(QiqHelpersClassResolver::class.java) + } +} diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inspection/QiqHelperInspectionSuppressor.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inspection/QiqHelperInspectionSuppressor.kt index 85b2c8d..bc55658 100644 --- a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inspection/QiqHelperInspectionSuppressor.kt +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/inspection/QiqHelperInspectionSuppressor.kt @@ -6,6 +6,7 @@ import com.intellij.psi.PsiElement import com.intellij.psi.util.PsiTreeUtil import com.jetbrains.php.lang.psi.elements.FunctionReference import io.github.jingu.idea_qiq_plugin.helper.QiqHelperRegistry +import io.github.jingu.idea_qiq_plugin.helper.QiqHelpersClassResolver import io.github.jingu.idea_qiq_plugin.lang.QiqInjectionSupport /** @@ -37,7 +38,11 @@ class QiqHelperInspectionSuppressor : InspectionSuppressor { if (!QiqInjectionSupport.isInQiqFile(call)) return false val name = call.name?.takeIf { it.isNotEmpty() } ?: return false - return QiqHelperRegistry.getInstance(element.project).resolveFqn(name) != null + val project = element.project + // Suppress for either resolution path: 1.x HelperLocator registration + // or a 2.x/3.x Qiq\Helpers subclass method. + return QiqHelperRegistry.getInstance(project).resolveFqn(name) != null || + QiqHelpersClassResolver.getInstance(project).hasHelper(name) } override fun getSuppressActions(element: PsiElement?, toolId: String): Array = diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/navigation/QiqHelperGotoDeclarationHandler.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/navigation/QiqHelperGotoDeclarationHandler.kt index 835ee29..11d31b1 100644 --- a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/navigation/QiqHelperGotoDeclarationHandler.kt +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/navigation/QiqHelperGotoDeclarationHandler.kt @@ -8,6 +8,7 @@ import com.intellij.psi.util.PsiTreeUtil import com.jetbrains.php.lang.psi.elements.FunctionReference import com.jetbrains.php.lang.psi.elements.PhpClass import io.github.jingu.idea_qiq_plugin.helper.QiqHelperRegistry +import io.github.jingu.idea_qiq_plugin.helper.QiqHelpersClassResolver import io.github.jingu.idea_qiq_plugin.lang.QiqInjectionSupport /** @@ -48,15 +49,23 @@ class QiqHelperGotoDeclarationHandler : GotoDeclarationHandler { if (!QiqInjectionSupport.isInQiqFile(call)) return null val name = call.name?.takeIf { it.isNotEmpty() } ?: return null - val classes = QiqHelperRegistry.getInstance(call.project).resolveClasses(name) + val project = call.project + + // 1.x HelperLocator style: name -> invokable Qiq\Helper\ class. + val classes = QiqHelperRegistry.getInstance(project).resolveClasses(name) + // 2.x/3.x style: name -> public method on a Qiq\Helpers subclass. + val methods = QiqHelpersClassResolver.getInstance(project).resolve(name) + if (log.isDebugEnabled) { - log.debug("Qiq helper goto: name='$name' resolvedClasses=${classes.size}") + log.debug("Qiq helper goto: name='$name' locatorClasses=${classes.size} helperMethods=${methods.size}") } - if (classes.isEmpty()) return null + if (classes.isEmpty() && methods.isEmpty()) return null + val targets = mutableListOf() // Prefer the __invoke method — the actual call target Qiq dispatches // to — falling back to the class declaration when none is declared. - val targets = classes.map { phpClass -> invokeMethodOf(phpClass) ?: phpClass } + classes.forEach { phpClass -> targets.add(invokeMethodOf(phpClass) ?: phpClass) } + targets.addAll(methods) return targets.toTypedArray() } From 5f2823cb1a82081a0f7d0992d85bf6c20baebf6c Mon Sep 17 00:00:00 2001 From: Yoshitaka Jingu Date: Fri, 29 May 2026 15:02:48 +0900 Subject: [PATCH 3/4] test: cover Qiq 2.x/3.x helper method filtering Extract the per-class helper-method discriminator into a pure QiqHelpersClassResolver.helperMethodsOf(PhpClass) companion function and unit-test it against in-memory PHP PSI: public/non-static/non-magic own methods of a user subclass are kept, members and classes under the \Qiq\ namespace are excluded. The PhpIndex processAllSubclasses walk stays a thin platform call; testing it would require an indexed fixture project (BasePlatformTestCase), which trips the platform light-project leak detector under the JUnit vintage engine, so the index-backed traversal is left to manual verification. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../helper/QiqHelpersClassResolver.kt | 38 +++--- .../helper/QiqHelpersClassResolverTest.kt | 110 ++++++++++++++++++ 2 files changed, 133 insertions(+), 15 deletions(-) create mode 100644 src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolverTest.kt diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolver.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolver.kt index c7396fb..7bc4e63 100644 --- a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolver.kt +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolver.kt @@ -8,6 +8,7 @@ import com.intellij.psi.util.CachedValuesManager import com.intellij.psi.util.PsiModificationTracker import com.jetbrains.php.PhpIndex import com.jetbrains.php.lang.psi.elements.Method +import com.jetbrains.php.lang.psi.elements.PhpClass import com.jetbrains.php.lang.psi.elements.PhpModifier /** @@ -50,32 +51,39 @@ class QiqHelpersClassResolver(private val project: Project) { val index = PhpIndex.getInstance(project) val result = mutableMapOf>() index.processAllSubclasses(HELPERS_BASE_FQN) { phpClass -> - // Only user-defined subclasses contribute; library helper classes - // (\Qiq\Helpers, \Qiq\Helper\Html\HtmlHelpers, ...) are covered by - // the runtime stub already. - if (!phpClass.fqn.startsWith(QIQ_NAMESPACE_PREFIX)) { - for (method in phpClass.ownMethods) { - if (isHelperMethod(method)) { - result.getOrPut(method.name) { mutableListOf() }.add(method) - } - } + for (method in helperMethodsOf(phpClass)) { + result.getOrPut(method.name) { mutableListOf() }.add(method) } true } return result } - private fun isHelperMethod(method: Method): Boolean = - method.access == PhpModifier.Access.PUBLIC && - !method.isStatic && - !method.isAbstract && - !method.name.startsWith("__") - companion object { private const val HELPERS_BASE_FQN = "\\Qiq\\Helpers" private const val QIQ_NAMESPACE_PREFIX = "\\Qiq\\" fun getInstance(project: Project): QiqHelpersClassResolver = project.getService(QiqHelpersClassResolver::class.java) + + /** + * The public helper methods a single `Qiq\Helpers` subclass + * contributes. Library classes under `\Qiq\` contribute nothing + * (their built-ins resolve via the runtime stub); only public, + * non-static, non-magic own methods of user classes qualify. + * + * Pure PSI logic (no index access) so it can be unit-tested against + * an in-memory file. + */ + fun helperMethodsOf(phpClass: PhpClass): List { + if (phpClass.fqn.startsWith(QIQ_NAMESPACE_PREFIX)) return emptyList() + return phpClass.ownMethods.filter(::isHelperMethod) + } + + private fun isHelperMethod(method: Method): Boolean = + method.access == PhpModifier.Access.PUBLIC && + !method.isStatic && + !method.isAbstract && + !method.name.startsWith("__") } } diff --git a/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolverTest.kt b/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolverTest.kt new file mode 100644 index 0000000..fef6cf2 --- /dev/null +++ b/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelpersClassResolverTest.kt @@ -0,0 +1,110 @@ +package io.github.jingu.idea_qiq_plugin.helper + +import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiFile +import com.intellij.psi.PsiFileFactory +import com.intellij.psi.util.PsiTreeUtil +import com.intellij.testFramework.junit5.RunInEdt +import com.intellij.testFramework.junit5.impl.TestApplicationExtension +import com.intellij.testFramework.junit5.resources.ProjectExtension +import com.jetbrains.php.lang.PhpFileType +import com.jetbrains.php.lang.psi.elements.PhpClass +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import org.junit.jupiter.api.extension.RegisterExtension +import kotlin.test.assertEquals + +/** + * Unit-tests the per-class filter that powers Qiq 2.x/3.x helper + * discovery: which methods of a `Qiq\Helpers` subclass count as helpers. + * + * The PhpIndex subclass walk (`processAllSubclasses`) is a thin platform + * call and is not exercised here; the discriminating logic + * (`QiqHelpersClassResolver.helperMethodsOf`) is pure PSI and runs against + * an in-memory file, so no indexed fixture is required. + */ +@RunInEdt +@ExtendWith(TestApplicationExtension::class) +class QiqHelpersClassResolverTest { + + companion object { + @JvmField + @RegisterExtension + val projectExtension = ProjectExtension() + } + + @Test + fun keepsOnlyPublicNonStaticNonMagicMethodsOfUserSubclass(project: Project) { + val names = helperNames( + project, + """ + { + var names: Set = emptySet() + ApplicationManager.getApplication().runReadAction { + val file: PsiFile = PsiFileFactory.getInstance(project) + .createFileFromText("Helpers.php", PhpFileType.INSTANCE, php) + val phpClass = PsiTreeUtil.collectElementsOfType(file, PhpClass::class.java).single() + names = QiqHelpersClassResolver.helperMethodsOf(phpClass).map { it.name }.toSet() + } + return names + } +} From a4d03da53998e3c64b94362a10fc173b4aa84847 Mon Sep 17 00:00:00 2001 From: Yoshitaka Jingu Date: Fri, 29 May 2026 15:22:12 +0900 Subject: [PATCH 4/4] fix: address PR review on helper registry scanning - QiqHelperRegistry.bodyNewExpressionFqn: only treat a `new X(...)` as the factory's return when it is the *direct* returned value (return argument for closures, direct body child for arrow functions). Avoids false positives such as `return foo(new X())` or a `new` assigned to a local but not returned. - Drop unused VirtualFile import. - File chooser filter: call equals on the non-null literal so the nullable VirtualFile.extension can't be dereferenced. - Add regression tests for the nested/non-returned new-expression cases. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../helper/QiqHelperRegistry.kt | 20 ++--- .../settings/QiqProjectConfigurable.kt | 2 +- .../helper/QiqHelperRegistryTest.kt | 78 +++++++++++++++++++ 3 files changed, 90 insertions(+), 10 deletions(-) diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistry.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistry.kt index a6ec911..7b2559b 100644 --- a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistry.kt +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistry.kt @@ -3,7 +3,6 @@ package io.github.jingu.idea_qiq_plugin.helper import com.intellij.openapi.components.Service import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.project.Project -import com.intellij.openapi.vfs.VirtualFile import com.intellij.psi.PsiFile import com.intellij.psi.PsiManager import com.intellij.psi.util.PsiTreeUtil @@ -189,19 +188,22 @@ class QiqHelperRegistry(private val project: Project) { } private fun bodyNewExpressionFqn(func: Function): String? { + // Regular closures: a `return new X(...)` whose returned value is + // *directly* a new-expression. Checking the return argument (not any + // descendant) avoids matching `return foo(new X())` or + // `return $svc->make(new X())`, which do not return X. val returns = PsiTreeUtil.findChildrenOfType(func, PhpReturn::class.java) .filter { PsiTreeUtil.getParentOfType(it, Function::class.java) === func } for (ret in returns) { - val new = PsiTreeUtil.findChildOfType(ret, NewExpression::class.java) ?: continue - classRefFqn(new.classReference)?.let { return it } + (ret.argument as? NewExpression)?.let { new -> + classRefFqn(new.classReference)?.let { return it } + } } - // Arrow functions: no PhpReturn — the body expression itself is the - // value. Search shallowly within the function for a NewExpression - // whose nearest enclosing Function is this arrow function. - val arrowNews = PsiTreeUtil.findChildrenOfType(func, NewExpression::class.java) - .filter { PsiTreeUtil.getParentOfType(it, Function::class.java) === func } - for (new in arrowNews) { + // Arrow functions (`fn () => new X(...)`): no PhpReturn — the body + // expression is the value. Only a *direct* child new-expression + // counts, so `fn () => foo(new X())` is not matched. + PsiTreeUtil.getChildOfType(func, NewExpression::class.java)?.let { new -> classRefFqn(new.classReference)?.let { return it } } return null diff --git a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqProjectConfigurable.kt b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqProjectConfigurable.kt index fbff7af..686e236 100644 --- a/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqProjectConfigurable.kt +++ b/src/main/kotlin/io/github/jingu/idea_qiq_plugin/settings/QiqProjectConfigurable.kt @@ -86,7 +86,7 @@ class QiqProjectConfigurable(private val project: Project) : BoundSearchableConf private fun chooseBootstrapFiles() { val descriptor = FileChooserDescriptorFactory.createMultipleFilesNoJarsDescriptor() .withTitle(QiqBundle.message("settings.qiq.helper.bootstrap.chooser.title")) - .withFileFilter { it.extension.equals("php", ignoreCase = true) } + .withFileFilter { "php".equals(it.extension, ignoreCase = true) } val toSelect = project.basePath?.let { com.intellij.openapi.vfs.LocalFileSystem.getInstance().findFileByPath(it) } FileChooser.chooseFiles(descriptor, project, toSelect) { files -> diff --git a/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistryTest.kt b/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistryTest.kt index a03900b..3f10367 100644 --- a/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistryTest.kt +++ b/src/test/kotlin/io/github/jingu/idea_qiq_plugin/helper/QiqHelperRegistryTest.kt @@ -208,6 +208,84 @@ class QiqHelperRegistryTest { assertEquals(0, map.size, "Indirect factories must not be indexed, got: $map") } + @Test + fun ignoresNewExpressionNestedInsideReturnedCall(project: Project) { + // `return foo(new Inner())` returns foo()'s result, not Inner — the + // scanner must not index 'wrap' as returning Inner. + val map = scan( + project, + """ + set('wrap', static function () use (${'$'}svc) { + return ${'$'}svc->make(new Inner()); + }); + } + } + """.trimIndent(), + ) + assertEquals(0, map.size, "new nested in a returned call must not be indexed, got: $map") + } + + @Test + fun ignoresNewExpressionThatIsNotTheReturnedValue(project: Project) { + // A `new` assigned to a local but not returned must not be indexed. + val map = scan( + project, + """ + set('proxy', static function () use (${'$'}svc) { + ${'$'}tmp = new Tmp(); + return ${'$'}svc->make(); + }); + } + } + """.trimIndent(), + ) + assertEquals(0, map.size, "non-returned new must not be indexed, got: $map") + } + + @Test + fun ignoresNewExpressionNestedInsideArrowReturnedCall(project: Project) { + // `fn () => foo(new Inner())` — Inner is not the arrow's value. + val map = scan( + project, + """ + set('wrap', static fn () => ${'$'}svc->make(new Inner())); + } + } + """.trimIndent(), + ) + assertEquals(0, map.size, "new nested in an arrow's returned call must not be indexed, got: $map") + } + @Test fun extractsMultipleRegistrationsFromProviderStyleFile(project: Project) { val map = scan(