ALT-10995

[adkozlov]

No description provided.

[meanmail]

Code Review

.github/workflows/publish.yaml

Line 4: push trigger without branch filter

Changing from pull_request to push without specifying branches means this workflow will run on every push to every branch, including main. If this is intentional for debugging CI — OK, but consider adding a branch filter before merging to main:

on:
  push:
    branches:
      - main

Or if needed for all branches, at least exclude dependabot/** and similar.

Also, the step name on line 43 still says "Upload plugin to PR" — but now the trigger is push, not pull_request, so the name is misleading.

---

intellij-plugin/hs-Python/src/org/hyperskill/academy/python/learning/PyEduUtils.kt

Lines 155-164: invokeLater + findPsiFile without runReadAction

findPsiFile (from com.intellij.openapi.vfs.findPsiFile) internally calls PsiManager.getInstance(project).findFile(this) which requires read access. Running on EDT does not guarantee a read lock in newer platform versions.

The old code had the same issue, but since this block is being refactored, it would be good to wrap the PSI access:

ApplicationManager.getApplication().invokeLater({
  runReadAction {
    val editorManager = FileEditorManager.getInstance(project)
    val analyzer = DaemonCodeAnalyzerEx.getInstanceEx(project)
    editorManager.openFiles
      .asSequence()
      .mapNotNull { it.findPsiFile(project) }
      .forEach { analyzer.cleanFileLevelHighlights(Pass.LOCAL_INSPECTIONS, it) }
  }
}, project.disposed)

Note: verify that cleanFileLevelHighlights doesn't require EDT-only (no read lock) — if it does, you'd need to split the read and write parts.

Lines 160-163: asSequence() on a small array

editorManager.openFiles returns a small array (typically < 20 files). Using asSequence() here adds overhead without meaningful benefit. A simple forEach chain would be cleaner:

for (file in editorManager.openFiles) {
  val psiFile = file.findPsiFile(project) ?: continue
  analyzer.cleanFileLevelHighlights(Pass.LOCAL_INSPECTIONS, psiFile)
}

This is a minor style nit, not blocking.

---

Overall

The refactoring is a clear improvement — removing the unnecessary module loop and hasOpenFiles() check, and properly deferring to EDT via invokeLater. The project.disposed condition is correct.

Main actionable item: wrap findPsiFile in runReadAction.

[adkozlov]

The push change was experimental, I wasn't able to understand why the workflow wasn't running.
Apparently, it was just a queue issue.

[adkozlov]

asSequence was used not to create an intermediate list. And to run analysis straight away.

[adkozlov]

invokeLater is guaranteed to run with a Write intent lock, see com/intellij/openapi/application/Application.java:405.
So an explicit read action can be omitted here.

[meanmail]

LGTM. Thanks for the clarifications:

• invokeLater runs under Write Intent lock — confirmed in Application.java javadoc, so explicit runReadAction is indeed unnecessary.
• asSequence — fair point about avoiding intermediate list creation.
• push trigger — understood it was experimental. Just make sure to scope it (or revert) before merging to main.

The refactoring is clean: unnecessary module loop removed, hasOpenFiles() guard eliminated, properly deferred to EDT. 👍