Skip to content

Conversation

@jherkel
Copy link
Contributor

@jherkel jherkel commented Sep 16, 2025

Original issue #5024

This fixes a generics angle bracket highlighting. It works for a multichar token like GTGT and GTGTGT. It ignores logic and arithmetic operators. As we need to distinguish between these two cases, java parser is used for this check.
I added also unit tests.
There is a similar commit #8500 but I don't think it can work in this way and there is no other activity since May 2025.

@mbien mbien linked an issue Sep 17, 2025 that may be closed by this pull request
@mbien mbien added Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Editor ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Sep 17, 2025
@apache apache locked and limited conversation to collaborators Sep 17, 2025
@apache apache unlocked this conversation Sep 17, 2025
@mbien
Copy link
Member

mbien commented Sep 17, 2025

you formatted the whole file unfortunately :(

We typically avoid doing unnecessary whitespace changes and limit them to the rewritten sections. (A few fixes here and there are ok, e.g same method)

@jherkel
Copy link
Contributor Author

jherkel commented Sep 17, 2025

Sorry for that. I started with some debuging and some experiments and unfortunately I had a different formatting options. So after some time I realized that I had something that works but the source file was full of debug outputs and temporary code with a wrong formatting. So I tried to reformat the whole code as an attempt to fix it...

@matthiasblaesing
Copy link
Contributor

For anyone wanting to have a look, this branch has an additional commit revering the unnessary whitespace changes: https://github.com/matthiasblaesing/netbeans/tree/pr-8834

Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

@jherkel could you please check my referenced branch:

  • 4f9a93e and c0bcae2 cleanup the unnessary changes
  • b92297c adjust two names so that they reflect the intented usage
  • c77d09e use correct return (null for no match)
  • a1e5f0e adds minimal test cleanup

From my POV these could be squashed into the initial commit to reduce the differences and make it clearer to read.

A quick test was successful and this looks good to me. The added tests looks sane, but a negative test with bitshift/comparison operators would be good.

@matthiasblaesing
Copy link
Contributor

Outside this PR was the comment:

c77d09e use correct return (null for no match)

I chose this {-1,-1} because I didn't want to change the color of '<' or '>' characters. So I checked the Netbeans sources and I found out that there is {-1,-1} used for an invalid span (it works, but I'm not 100% sure if it is correct). Because null here is used when an opening or a closing element doesn't exist.

Ok, I understand. That needs a comment though if kept. The correct solution would be to return null from findOrigin for the cases where you would return {-1,-1} from findMatches.

Cite from findOrigin:

Returns:
The starting and ending offset of the original area or null if the matcher can't detect the origin area within the lookahead distance. If null is returned the infrastructure will never call the findMatches method on this instance.

move generics check to findOrigin method. Fix wrong reformatting and code cleanup (Matthias Bläsing)
@matthiasblaesing
Copy link
Contributor

@lahodaj @mbien this looks sane to me. Would you mind having a look?

Comment on lines +165 to +175
if (token.id().equals(JavaTokenId.GTGT)) {
if (backward) {
list = th.tokenSequenceList(seq.languagePath(), 0, originOffset - token.length() + 1);
}
counter += originOffset - seq.offset();
} else if (token.id().equals(JavaTokenId.GTGTGT)) {
if (backward) {
list = th.tokenSequenceList(seq.languagePath(), 0, originOffset - token.length() + 1);
}
counter += originOffset - seq.offset();
}
Copy link
Member

Choose a reason for hiding this comment

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

both branches are the same?

given this snippet:

        int n1 = 10 >> 1;
        int n2 = 10>> 1;

        int n3 = 10 >>> 1;
        int n4 = 10>>> 1;

n2 and n4 would try to match opening brackets, while n1 and n3 work which have a space before the first >.

image

also reproducible with the << counterpart

Copy link
Contributor

Choose a reason for hiding this comment

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

Reproducible problem with this full source:

public class AA {
    public static void main(String[] args) {
        int n1 = 10 >> 1;
        int n2 = 10>> 1;
    }
}

The issue is, that the javac gives us kind INT_LITERAL when you query it for the position of the third > (the first in the fourth line).

@jherkel quick idea for a test would be:

    @Test
    public void testAngleBrackets() throws Exception {
        // original tests
        assertHasNoOrigin("int n1 = 10 ^>> 1;");
        assertHasNoOrigin("int n1 = 10 >^> 1;");
        assertHasNoOrigin("int n1 = 10 >>^ 1;");
        assertHasNoOrigin("int n1 = 10^>> 1;"); // This is the only failing one here
        assertHasNoOrigin("int n1 = 10>^> 1;");
        assertHasNoOrigin("int n1 = 10>>^ 1;");
        // test other esoteric variants
    }

    /**
     * Pass a method body as {@code angleStr}, where the caret position is
     * marked by {@code ^}. The method checks, that the
     * JavaBracesMatcher#findOrigin returns null.
     *
     * @param angleStr
     * @throws Exception 
     */
    private void assertHasNoOrigin(String angleStr) throws Exception {
        testNumber++;
        String srcTmp = makeTestClass(angleStr);
        int caretPos = srcTmp.indexOf('^');
        String sourceCode = srcTmp.substring(0, caretPos) + srcTmp.substring(caretPos + 1);
        FileObject wd = FileUtil.toFileObject(getWorkDir());
        FileObject sourceDir = FileUtil.createFolder(wd, "src");
        FileObject buildDir = FileUtil.createFolder(wd, "build");
        FileObject cacheFolder = FileUtil.createFolder(wd, "cache");
        Paths.get(cacheFolder.toURI()).toFile().mkdirs();
        FileObject testFO = FileUtil.createData(sourceDir, "test/Test" + testNumber + ".java");
        TestUtilities.copyStringToFile(testFO, sourceCode);
        SourceUtilsTestUtil.prepareTest(sourceDir, buildDir, cacheFolder);
        JavaSource source = JavaSource.forFileObject(testFO);
        assertNotNull(source);
        DataObject od = DataObject.find(testFO);
        EditorCookie ec = od.getCookie(EditorCookie.class);
        Document doc = ec.openDocument();
        doc.putProperty(Language.class, JavaTokenId.language());
        doc.putProperty("mimeType", JavaKit.JAVA_MIME_TYPE);
        BracesMatcherFactory factory = new JavaBracesMatcher();
        MatcherContext context = BracesMatchingTestUtils.createMatcherContext(doc, caretPos, false, 1);
        BracesMatcher matcher = factory.createMatcher(context);
        assertNull(matcher.findOrigin());
    }

Comment on lines +358 to +360
javaSource.runUserActionTask(new Task<CompilationController>() {
@Override
public void run(CompilationController ctrl) throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

feel free to use a lambda if you want, a bit less noisy

@mbien mbien added this to the NB29 milestone Nov 10, 2025
@jherkel
Copy link
Contributor Author

jherkel commented Nov 16, 2025

I checked source code and the problem here is that as @matthiasblaesing mentioned a parser returns INT_LITERAL for offset position 139 that contains '>' character. Output from lexers is:

T[50]: "int" F(3) INT[29] FlyT, la=1, @2bb62414
T[51]: " " F(1) WHITESPACE[124] FlyT, la=1, @6aecbb8d
T[52]: "n1" <132,134> IDENTIFIER[1] DefT, la=1, @51ac12ac
T[53]: " " F(1) WHITESPACE[124] FlyT, la=1, @6aecbb8d
T[54]: "=" F(1) EQ[84] FlyT, la=1, @565b064f
T[55]: " " F(1) WHITESPACE[124] FlyT, la=1, @6aecbb8d
T[56]: "10" <137,139> INT_LITERAL[64] DefT, la=1, @1fb2eec
*[57]: ">>" F(2) GTGT[108] FlyT, la=1, @1698fc68
T[58]: " " F(1) WHITESPACE[124] FlyT, la=1, @6aecbb8d
T[59]: "1" <142,143> INT_LITERAL[64] DefT, la=1, @463afa6e
T[60]: ";" F(1) SEMICOLON[80] FlyT, @25b2cfcb

Important line is T[56]. It has a position [137-139) (close-open interval) and from my point of view it is correct. But as for above mentioned, javac returns INT_LITERAL. I tried to debug source code (TreeUtilities.java 402-433 lines) and I'm not sure about this line
if (startPos < pos && endPos >= pos) {

If I understand correctly, this checks if current three path encloses offset position. But If the position interval is close-open maybe this condition should be
if (startPos <= pos && endPos > pos) {

But I'm not sure here. Maybe someone who understands javac internals can clarify this. If I used the second version I could see correct behavior for "int n2 = 10>> 1;" but some unit tests for Java Source Base failed.
Right now I'm not sure what exactly is wrong. Does anybody have anu idea?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Editor Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add generics angle brackets (<>) highlighting for Java

3 participants