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

Fix wrong intention when referencing parameters in static fields #2886

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SirYwell
Copy link
Contributor

@SirYwell SirYwell commented Dec 3, 2024

Local classes (and interfaces) can reference parameters from the enclosing method, however not in static contexts.
However, IntelliJ currently incorrectly suggests making the parameter static. This should be avoided.

As PsiParameter also covers variables in for-each loops, pattern variables, and catch variables, those were affected too.

Example:

public class StaticReference {

    void m(int p) {
        int lv;
        if (null instanceof Object tp) {
            for (Object ef : new Object[0]) {
                class C {
                    static int LV = lv; // works for local variables correctly, everything else gets the wrong intention
                    static int P = p;
                    static Object TP = tp;
                    static Object EF = ef;
                }
            }
        }
    }
}

@BartvHelvert BartvHelvert self-assigned this Jan 25, 2025
@BartvHelvert
Copy link
Member

@SirYwell thanks for the PR, could you maybe create an issue for it in our issue tracker, also could you add a test?

@BartvHelvert BartvHelvert self-requested a review January 25, 2025 22:54
@BartvHelvert BartvHelvert removed their assignment Jan 25, 2025
@SirYwell
Copy link
Contributor Author

@SirYwell thanks for the PR, could you maybe create an issue for it in our issue tracker, also could you add a test?

Thanks, I created https://youtrack.jetbrains.com/issue/IDEA-366531/. I didn't find any related tests, where would they be placed best?

@BasLeijdekkers
Copy link
Member

@SirYwell take a look at the com.intellij.codeInsight.daemon.impl.quickfix.ModifierTest class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants