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

Error IDs for problems unique to javac #929

Open
wants to merge 702 commits into
base: dom-with-javac
Choose a base branch
from

Conversation

datho7561
Copy link

@datho7561 datho7561 commented Nov 7, 2024

This causes 6 regressions because 'possible this escape' is now reported.

@datho7561 datho7561 force-pushed the dom-with-javac-javac-problemid branch from 3c36743 to 1ca5e76 Compare November 7, 2024 14:49
@mickaelistria
Copy link

Thanks for adding it. Would you please try also submitting the new problem definition to JDT upstream? I'm curious about whether JDT would welcome some ids.

@datho7561 datho7561 force-pushed the dom-with-javac-javac-problemid branch from 1ca5e76 to 6bd83c4 Compare November 7, 2024 16:00
@datho7561
Copy link
Author

Would you please try also submitting the new problem definition to JDT upstream?

Do you mean submitting the IDs to IProblem? I think submitting them to IProblem would be a good idea.

I think ECJ/jdt.core will eventually need to implement the this escape check. My understanding is checking for this escape is important for value classes. I think that the other warnings would also be interesting to implement.

testforstephen and others added 24 commits November 12, 2024 11:19
eclipse-jdt#663)

* Fixes test0063, test0064, test0065, and more - getQualifiedName expects no generics for type declarations

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

Cleanup debugging text

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

Bad rebase

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

* Fix some regressions

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

* Problems determing when a type is or is not generic, and regressions

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

---------

Signed-off-by: Rob Stryker <stryker@redhat.com>
Co-authored-by: Rob Stryker <stryker@redhat.com>
…tructor

- Add `parentType` to JavacMethodBinding
  - Currently only used when requesting the method binding through
    `typeBinding.getDeclaredMethods`
- Fix implementation of `JavacTypeBinding.getName` for wildcard types
- In `JavacMethodBinding.getKey()`, prefer using the return type from `methodType` to the return type from `methodSymbol`
- Use `JavacMethodBinding.parentType` in `getKey()`
  - Note that the erasure is intentionally not used.

Signed-off-by: David Thompson <davthomp@redhat.com>
…InDefaultConstructor based on whether the diagnostics constructor is generated or not (eclipse-jdt#691)
As this cause javac to fail with "IllegalArgument error: option --system
cannot be used together with --release"
If you are extending a class and haven't implemented an abstract method
that contains an argument that's an array type,
and the array element type is a well-known type such as `java.lang.Object`,
this will now properly use the suggested name from JDT.

eg. parent signature `abstract public void useArray(Object[] arg1)`

child signature `public void useArray(Object[] o)`

`o` is the JDT suggested name for objects.

Signed-off-by: David Thompson <davthomp@redhat.com>
Also fix the binding key and display name for capture bindings

Signed-off-by: David Thompson <davthomp@redhat.com>
- This also fixes the quickfix as a result

eg.

```java
Function<Integer, Integer> func = x -> {
    System.out.println(x);
};
```

becomes

```java
Function<Integer, Integer> func = x -> {
    System.out.println(x);
    return x;
};
```

Signed-off-by: David Thompson <davthomp@redhat.com>
Different versions of Java allow different modifiers on interface methods,
so ECJ generates slightly different problem ids for each of these cases.
Use the compiler settings to determine which to use.
This affects the logic of the quick fixes,
so it should fix some jdt-ls test cases.

Signed-off-by: David Thompson <davthomp@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>

Fix regression

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

Fix regressions

Signed-off-by: Rob Stryker <stryker@redhat.com>
mickaelistria and others added 22 commits November 12, 2024 11:21
At some point the problem id that get ignored changed from 0 to -1 but
this warning code was missed in the conversion.
This saves a warning in every file with license header.
- Perform prefix validation for keywords
- Suggest `super` when a statement is expected

Signed-off-by: David Thompson <davthomp@redhat.com>
Trim off the leading `*` when working on a method reference that spans
multiple Javadoc lines.

Signed-off-by: David Thompson <davthomp@redhat.com>
- Change dom conversion logic to recover incomplete type names properly
- Restrict the suggested classes based on if they are a class or
  interface
- Do not suggest final classes
- Do not suggest the current class

Signed-off-by: David Thompson <davthomp@redhat.com>
Makes changes to what fields in completion results we set
and what we set them to,
with the goal of reducing test failures

Signed-off-by: David Thompson <davthomp@redhat.com>
IProblem.UnsafeReturnTypeOverride seems the correct one.
These are compiler.note.removal.filename,
compiler.note.deprecated.plural.additional and the fact that they don't
have position info in them makes them not suitable to create Problems
for them.
- recover `new ` in AST converter
- some fixes in bindings to get some test cases working
- scan backwards for first non-whitespace character when performing AST
  search
- use type hierachy to build a list of potential constructors
  - This method is expensive but accurate
- suggest creating anonymous classes when completing constructors for interfaces
- always provide all constructors of current type to be bug compatible
  with existing CompletionEngine
- use diamond operators

Limitations:
- relevance numbers are not quite right yet
- generic types don't quite work as expected

Signed-off-by: David Thompson <davthomp@redhat.com>
Make sure that calling substring is done with acceptable values, values
bigger than string length or negative ones are plain wrong.
```
!ENTRY org.eclipse.jdt.core 4 0 2024-10-31 11:45:52.859
!MESSAGE Failed to convert Javadoc
!STACK 0
java.lang.StringIndexOutOfBoundsException: Index 2912 out of bounds for
length 1736
```
eg.

```java
import java.util.List;

/**
 * Like a {@link List} if it wasn't a list.
 */
public class NotAList {
}
```

The `java.util.List` import is needed to distiguish the Javadoc mention
of `List` from other potential `List` classes.

Signed-off-by: David Thompson <davthomp@redhat.com>
- set prefix for `this.t|` completions correctly
- improve extends and implements completion with keywords
eg. avoid generic completion results and instead recommend keywords here:

```java
public class MyClass | {
	// ...
}
```

Signed-off-by: David Thompson <davthomp@redhat.com>
- Fix completion issue with `myVariable.|\nMyObj asdf = null;`
- Fix for `myTarget.|` completion

Signed-off-by: David Thompson <davthomp@redhat.com>
Reduce the parsed AST before resolving to get a performance boost.
The following cases are fixed:
* Interface methods
* Abstract methods
* Methods marked with Override annotation
- `myMethod().|`
- completion after method declaration in `@interface`

Signed-off-by: David Thompson <davthomp@redhat.com>
Recover the prefix in this case:

```java
public class MyClass ext| {
}
```

So that we know to only suggest `extends` and not `implements`.

Also, do not suggest `implements` for interfaces.

Signed-off-by: David Thompson <davthomp@redhat.com>
- fix duplicates in annotation members
- fix range for annotation members
- fix type of annotation member completions
- opening completion in a normal annotation or single value annotation
  before `(`, eg `@MyAnnota|tion(value = 1)`,
  now tries to complete the annotation name rather than the annotation
  members

Fixes eclipse-jdt#930

Signed-off-by: David Thompson <davthomp@redhat.com>
- Javadoc will need to be revisited, but for now I turned off the
  default completion

Signed-off-by: David Thompson <davthomp@redhat.com>
This causes 6 regerssions because 'possible this escape' is now
reported.

Signed-off-by: David Thompson <davthomp@redhat.com>
@datho7561 datho7561 force-pushed the dom-with-javac-javac-problemid branch from 6bd83c4 to 0c6edc8 Compare November 12, 2024 14:28
@mickaelistria
Copy link

Do you mean submitting the IDs to IProblem? I think submitting them to IProblem would be a good idea.

Yes, that's what I have in mind too.

But that doesn't mean we cannot merge it in the meantime.

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.

7 participants