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

Dom with javac may13 #393

Merged

Conversation

robstryker
Copy link

No description provided.

Copy link

@mickaelistria mickaelistria left a comment

Choose a reason for hiding this comment

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

@datho7561 introduced canonicalized that seems to fix some issues as some tests (and probably some "functional" code does use identity comparison for bindings). Please use it whenever constructing a new binding.

RequiresDirective[] arr = this.moduleSymbol.getDirectives().stream().filter((x) -> x.getKind() == DirectiveKind.REQUIRES).map((x) -> (RequiresDirective)x).toArray(RequiresDirective[]::new);
IModuleBinding[] arr2 = new IModuleBinding[arr.length];
for( int i = 0; i < arr.length; i++ ) {
arr2[i] = new JavacModuleBinding((ModuleType)arr[i].module.type, this.resolver);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

public String[] getExportedTo(IPackageBinding packageBinding) {
ExportsDirective[] arr = this.moduleSymbol.getDirectives().stream().filter((x) -> x.getKind() == DirectiveKind.EXPORTS).map((x) -> (ExportsDirective)x).toArray(ExportsDirective[]::new);
for( int i = 0; i < arr.length; i++ ) {
JavacPackageBinding tmp = new JavacPackageBinding((PackageSymbol)arr[i].packge, this.resolver);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

OpensDirective[] arr = this.moduleSymbol.getDirectives().stream().filter((x) -> x.getKind() == DirectiveKind.OPENS).map((x) -> (OpensDirective)x).toArray(OpensDirective[]::new);
IPackageBinding[] arr2 = new IPackageBinding[arr.length];
for( int i = 0; i < arr.length; i++ ) {
arr2[i] = new JavacPackageBinding((PackageSymbol)arr[i].packge, this.resolver);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

UsesDirective[] arr = this.moduleSymbol.getDirectives().stream().filter((x) -> x.getKind() == DirectiveKind.USES).map((x) -> (UsesDirective)x).toArray(UsesDirective[]::new);
ITypeBinding[] arr2 = new ITypeBinding[arr.length];
for( int i = 0; i < arr.length; i++ ) {
arr2[i] = new JavacTypeBinding(arr[i].getService().type, this.resolver);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

ProvidesDirective[] arr = this.moduleSymbol.getDirectives().stream().filter((x) -> x.getKind() == DirectiveKind.PROVIDES).map((x) -> (ProvidesDirective)x).toArray(ProvidesDirective[]::new);
ITypeBinding[] arr2 = new ITypeBinding[arr.length];
for( int i = 0; i < arr.length; i++ ) {
arr2[i] = new JavacTypeBinding(arr[i].getService().type, this.resolver);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

JavacTypeBinding tmp = new JavacTypeBinding(arr[i].getService().type, this.resolver);
if(service.getKey().equals(tmp.getKey())) {
// we have our match
JavacTypeBinding[] ret = arr[i].getImplementations().stream().map(x -> new JavacTypeBinding((ClassType)x.type, this.resolver)).toArray(JavacTypeBinding[]::new);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

} catch (JavaModelException e) {
// TODO Auto-generated catch block
e.printStackTrace();
return null;
}
}

public IModuleBinding getModule() {
return new JavacModuleBinding(this.packageSymbol.modle, this.resolver);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

List<Type> z2 = ((ClassType)this.type).interfaces_field;
ArrayList<JavacTypeBinding> l = new ArrayList<>();
if( z1 != null ) {
l.add(new JavacTypeBinding(z1, this.resolver));

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

}
if( z2 != null ) {
for( int i = 0; i < z2.size(); i++ ) {
l.add(this.resolver.canonicalize(new JavacTypeBinding(z2.get(i), this.resolver)));

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

public IModuleBinding getModule() {
Symbol o = this.type.tsym.owner;
if( o instanceof PackageSymbol ps) {
return new JavacModuleBinding(ps.modle, this.resolver);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

Copy link

@mickaelistria mickaelistria left a comment

Choose a reason for hiding this comment

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

Some missing canonicalize.
But everything else seems good.

@@ -58,7 +59,8 @@ public JavacModuleBinding(final ModuleSymbol moduleSymbol, final ModuleType modu
@Override
public IAnnotationBinding[] getAnnotations() {
// TODO - don't see any way to get this?
return null; //new IAnnotationBinding[0];
List<Attribute.Compound> list = moduleSymbol.getRawAttributes();
return list.stream().map((x) -> new JavacAnnotationBinding(x, this.resolver, this)).toArray(JavacAnnotationBinding[]::new);

Choose a reason for hiding this comment

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

this needs also .map(resolver::canonicalize)

resolve();
JCTree javacNode = this.converter.domToJavac.get(typeParameter);
if (javacNode instanceof JCTypeParameter jcClassDecl) {
return new JavacTypeBinding(jcClassDecl.type, this);

Choose a reason for hiding this comment

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

This should call resolver.canonicalize(...)

ExportsDirective[] arr = this.moduleSymbol.getDirectives().stream().filter((x) -> x.getKind() == DirectiveKind.EXPORTS).map((x) -> (ExportsDirective)x).toArray(ExportsDirective[]::new);
IPackageBinding[] arr2 = new IPackageBinding[arr.length];
for( int i = 0; i < arr.length; i++ ) {
arr2[i] = new JavacPackageBinding((PackageSymbol)arr[i].packge, this.resolver);

Choose a reason for hiding this comment

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

this should be canonicalize(new ...)

Rob Stryker added 14 commits May 21, 2024 10:50
Signed-off-by: Rob Stryker <stryker@redhat.com>
…f for-loop

Signed-off-by: Rob Stryker <stryker@redhat.com>
…assignment

Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
…ensions

Signed-off-by: Rob Stryker <stryker@redhat.com>
…yield statement Part 1 and 2

Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
…ources

Signed-off-by: Rob Stryker <stryker@redhat.com>
@robstryker robstryker force-pushed the dom-with-javac-may13 branch from c2ea72d to 1c38bcc Compare May 21, 2024 14:51
Rob Stryker added 2 commits May 21, 2024 12:01
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
@robstryker
Copy link
Author

Merging without canocolization... we will revisit that. It's a good point but i've had some issues and am hesitant to add that part at this moment.

@robstryker robstryker merged commit 2e2d4d4 into eclipse-jdtls:dom-with-javac May 21, 2024
2 of 4 checks passed
@mickaelistria
Copy link

We will revisit it as part of #395

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.

2 participants