Skip to content

New unncessary code warnings due to not considered access restrictions #1998

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

Open
HannesWell opened this issue Apr 6, 2025 · 9 comments
Open

Comments

@HannesWell
Copy link
Member

On the master build, since today (yesterday was fine) there are ten new warnings, indicating that suppression of certain restriction warnings would not be necessary anymore, although this is necessary and also doesn't show up in the IDE when using the latest I-Build. Therefore I assume this is a false-positive:

Image

I'm resetting the quality-gate for the master to not further block unrelated PRs, but think that this should be investigate.
@iloveeclipse could this be a JDT issue?

@iloveeclipse
Copy link
Member

could this be a JDT issue?

I'm not aware about recent related JDT changes, also if IDE doesn't show them it looks like maven/tycho build problem? Could be some maven/tycho snapshot issue?

Here what was changed in last build: eclipse-platform/eclipse.platform.releng.aggregator@9de1608

@laeubi
Copy link
Contributor

laeubi commented Apr 6, 2025

I'm not aware about recent related JDT changes, also if IDE doesn't show them it looks like maven/tycho build problem? Could be some maven/tycho snapshot issue?

Tycho only calls JDT compiler it does not report any problems itself. In any case last commit was 2days ago... It could be of course that there are compiler changes not yet deployed, then IDE / Maven might use different things.

@iloveeclipse
Copy link
Member

Here is direct link to report https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/master/1079/eclipse/new/ where Unnecessary @SuppressWarnings("restriction") is reported for /org.eclipse.swt.tools.spies/src/org/eclipse/swt/tools/internal/Sleak.java

I'm not aware about recent related JDT changes, also if IDE doesn't show them it looks like maven/tycho build problem?

If I disable processing of SuppressWarnings annotations in the IDE via org.eclipse.jdt.core.compiler.problem.suppressWarnings=disabled

I see that few warnings are reported for Sleak, example:

Discouraged access: The type 'WidgetSpy.NonDisposedWidgetTracker' is not API (restriction on required project 'org.eclipse.swt.gtk.linux.x86_64')

Image

So the tycho build seem to calculate dependencies differently to PDE / see different access restrictions or no restgrictions at all?

@laeubi
Copy link
Contributor

laeubi commented Apr 7, 2025

So the tycho build seem to calculate dependencies differently to PDE

I'm not sure what this means, PDE+Tycho using different ways to compute the classpath, but of course this should not matter as I hope we do not have one provider that restricts access and another that don't.

see different access restrictions or no restgrictions at all?

The restrictions are computed doing an OSGi resolve (at Tycho) PDE uses omething similar as far as I know (but based on the Target state), but again in what case could this matter at all?

The one thing that might be confusing compiler here is:

  • org.eclipse.ui is a "friend" of swt.internal and has visibility:=reexport
  • org.eclipse.swt.tools.spies require-bundle org.eclipse.ui so get its imports from there because of the reexport

So it might depend on the ordering what restriction is seen first (or not).

@laeubi
Copy link
Contributor

laeubi commented Apr 7, 2025

I now created

to see if it makes a difference.

@laeubi
Copy link
Contributor

laeubi commented Apr 7, 2025

Okay this fails now, what makes me even wonder how it is supposed to work that SWT requires platform at all?

@iloveeclipse
Copy link
Member

Okay this fails now, what makes me even wonder how it is supposed to work that SWT requires platform at all?

SWT doesn't require platform.ui, but Sleak (from org.eclipse.swt.tools.spies) does.

@HannesWell
Copy link
Member Author

I'm not aware about recent related JDT changes, also if IDE doesn't show them it looks like maven/tycho build problem?

If I disable processing of SuppressWarnings annotations in the IDE via org.eclipse.jdt.core.compiler.problem.suppressWarnings=disabled

Maybe the build uses some different compiler settings with respect to warnings. IIRC we ignore some kind of warnings by default in the parent-pom respectively set it in the projects, but it's just a unverified guess.
In general something must have changed somewhere in the chain of tools, because I see no code changes that explain this.

Btw. I now also see similar new warnings in platform.ui:
https://ci.eclipse.org/platform/job/eclipse.platform.ui/job/master/947/eclipse/new/
Although these are new unused-code warnings regarding record components, so actually a different type of warning.

@iloveeclipse
Copy link
Member

Btw. I now also see similar new warnings in platform.ui:
https://ci.eclipse.org/platform/job/eclipse.platform.ui/job/master/947/eclipse/new/
Although these are new unused-code warnings regarding record components, so actually a different type of warning.

Those warnings about unused record parameters should be gone now with latest nightly build, please re-run. See eclipse-jdt/eclipse.jdt.core#3905

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

No branches or pull requests

3 participants