eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0
162 stars 130 forks source link

Incorrect Error markers for linked classpath folders with include patterns #646

Closed hborchardt closed 1 year ago

hborchardt commented 1 year ago

Hi, thanks for building the awesome eclipse!

Version: 2022-12 (4.26.0) Build id: 20221201-1913

I want to share a source folder between two projects. For that I configure the following sourceSet in gradle:

project(':project_a') {
  apply plugin: "java"
  sourceSets {
    main {
      java {
        srcDirs "src/main/java"
        srcDirs "../project_b/src/main/java"
        include "A/**"
        include "B/C/**"
      }
    }
  }

It builds fine and compiles and runs, but editing a file in A that references classes in package B.C shows error markers:

image

The error is "The import B cannot be resolved".

Interestingly, on the left, the A.java file does not have an error indicator, also the Problems tab does not show problems. I had a look in the code and it appears that the markers to be shown in the editor are taken from the project tree, and naively trying to write a compiler unit test for my use case also led to no markers, so this left me confused.

The markers do not appear when not specifying the include statements. The markers also do not appear when I write include "B/**", without the "C" subpackage, for the filter.

I am attaching a minimal reproduction example gradle project here: repo.zip

I would very much appreciate any insights into what is going on 🙂

iloveeclipse commented 1 year ago

To exclude anything not coming from "plain" JDT, can you please attach a plain Java project (no gradle/maven/whatever references / build scripts etc)? This would at least sort out some broken .classpath files or rewritten bin folders or whatever.

hborchardt commented 1 year ago

Sure thing, this one is just me clicking in eclipse: eclipse-workspace-bug.zip

hborchardt commented 1 year ago

Apparently someone had the same problem years ago: https://bugs.eclipse.org/bugs/show_bug.cgi?id=119161 The added test testIncludeCUOnly01_new: https://github.com/eclipse-jdt/eclipse.jdt.core/blob/7eeb9b4342d4d83f3a5a1d078a4e4dff0ab83bb2/org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/InclusionPatternsTests.java#L431 has no errors, because it runs with project option COMPILER_COMPLIANCE set to "1.4", which matters here: https://github.com/eclipse-jdt/eclipse.jdt.core/blob/c58f581d8cef2c38a339b8056b3f48f1d5365731/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java#L105-L107 and triggers different package resolution code paths here: https://github.com/eclipse-jdt/eclipse.jdt.core/blob/c58f581d8cef2c38a339b8056b3f48f1d5365731/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/SearchableEnvironment.java#L996-L1008

where matchingRoots in my case ends up being an empty array, due to this function: https://github.com/eclipse-jdt/eclipse.jdt.core/blob/7eeb9b4342d4d83f3a5a1d078a4e4dff0ab83bb2/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/NameLookup.java#L318-L320

Adding this.project.setOption(JavaCore.COMPILER_COMPLIANCE, "11"); to the beginning of the testIncludeCUOnly01_new test produces a failing test case for my scenario.

iloveeclipse commented 1 year ago

Great, thanks for the analysis. Looks like this is a duplicate of https://github.com/eclipse-jdt/eclipse.jdt.core/issues/485 and will be fixed by https://github.com/eclipse-jdt/eclipse.jdt.core/pull/490 .

I will add your proposed test too.

iloveeclipse commented 1 year ago

Looks like this is a duplicate of #485 and will be fixed by #490

Unfortunately I can't reproduce the problem with your attachments, so please verify the fix in the tomorrow nightly SDK build you will find at https://download.eclipse.org/eclipse/downloads/

hborchardt commented 1 year ago

I tested with the nightly SDK and the issue is resolved. It also works with my original multi-project gradle setup, so I am very happy!

Thanks a lot for moving forward the fix in the existing pull request.

iloveeclipse commented 1 year ago

Thanks for validation.