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

Completion test0123 v2 #1240

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

Conversation

robstryker
Copy link

Can't figure out how to re-open #1109

So made a new PR instead

datho7561 and others added 30 commits February 10, 2025 11:40
- 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>
Signed-off-by: David Thompson <davthomp@redhat.com>
- Do not reuse the context when compiling to multiple output directories
- Implement Alex's idea of a separate file of javac-specific error codes

Signed-off-by: David Thompson <davthomp@redhat.com>
work-in-progress:
- prefix filtering
- see if I can get the binding key bugs resolved

Fixes eclipse-jdt#938

Signed-off-by: David Thompson <davthomp@redhat.com>
In order to accomplish this,
I have changed the logic for adjusting the offset used for the node search.

This causes some regressions:
- A few cases where the test expects the AST not to be recovered but javac does recover it (eg. org.eclipse.jdt.core.tests.model.CompletionTests_1_5.test0197)
- @| in Javadoc is not handled. Before it got no completion due to not finding the correct node, but now it gets default completion, which causes some regressions. I'll fix these when dealing with @| properly
- The test cases sometimes expect the current class to be suggested in new | completion even when the type doesn't match, but sometimes they don't. I'll need to investigate further

Signed-off-by: David Thompson <davthomp@redhat.com>
When resolving, we do only need the symbols/bindings of depedencies.
Minimize the amount of option to skip eg parsing Javadoc for transitive
files that are only relevant as part of binding resolution.
- Both block and inline tags
- Take into account which ones are applicable to the declaration being
  Javadocumented

Fixes eclipse-jdt#948

Signed-off-by: David Thompson <davthomp@redhat.com>
as it's reused on the whole lifecyle of JDT.
* Also do not share the Platform filemanager with build (a bit more
isolation)
* a few other optimization to save CPU on irrelevant operations
Because resolving from .class deps is much faster than resolving from
.java deps
It's most likely useless to get linting when none of those are
requested.
datho7561 and others added 28 commits February 10, 2025 11:40
Should fix 6 cases without regressions,
because one of the projects used to test completion
has two classes called `SuperClass` in the
default package.

Signed-off-by: David Thompson <davthomp@redhat.com>
eg.

```java
public class MyClass extends MyOtherClass {
  public MyClass() {
    sup|
  }
}

class MyOtherClass {
  public MyOtherClass(int i) {}
  protected MyOtherClass(Object i) {}
  MyOtherClass(boolean b) {}
  private MyOtherClass(String s) {}
}
```

Requires eclipse-jdt#1206 in order to work properly if you have substring matching
enabled in your IDE.

Fixes eclipse-jdt#1205

Signed-off-by: David Thompson <davthomp@redhat.com>
- Do prefix filtering first; string comparison should be fast compared
  to more involved operations
- Fix bad implementation of `protected` member classes;
  `getParent()` wasn't the right method to get the parent class of a
  member class
- Impossible classes filtering was wrong, it was checking methods
  instead

Signed-off-by: David Thompson <davthomp@redhat.com>
Fixes:
`ASTConverterTest.test0113`
`ASTConverterTest.test0114`
`ASTConverterTest.test0115`

as well as tests for other Java versions

Note: we only need to accomodate Java >= 8 in the AST conversion process,
since that's what upstream does now.

Signed-off-by: David Thompson <davthomp@redhat.com>
Should fix 17 test cases

Signed-off-by: David Thompson <davthomp@redhat.com>
eg. `MyType\u005b] myTypeArray = null;`

Fixes 4 tests:
`ASTConverterTest.test0266` and the versions for other Java compliances

Signed-off-by: David Thompson <davthomp@redhat.com>
`ThisExpression` is it's own node, which we will need to handle
differently.

Should fix 2 test cases.

Signed-off-by: David Thompson <davthomp@redhat.com>
AST no longer supports it thus no need for Javac converter to be
complicated for nothing.
It's now taken into account for members of a qualified type being completed.

Signed-off-by: David Thompson <davthomp@redhat.com>
Failback to type of declaration from Search if not resolved
Not needed as neither Javac nor ECJ nor JDT makes use of anything pre
Java 8 nowadays.
VariableDeclaration.extraArrayDeclarataion is not supported in Java 8+
but was used without guards which shouldn't have happened.
Fixes eclipse-jdt#1224

Signed-off-by: David Thompson <davthomp@redhat.com>
- Fix the number part of IVariableBinding's key
  - This means that the bindings for different variables with the same
    name in the same method can be distinguished eg. `rob` and `i` can
    be distinguished:

```java
public class Main {
	public void myMethod() {
		for (int i = 0; i < 10; i++) {
			int rob = 12;
		}
		for (String i = "asdf"; "hjkl".equals(i); i = "hjkl") {
			String rob = "rob";
		}
	}
}
```

- As a result, completion works deterministically in these cases
  (eg. before you just had to hope that the correct binding was cached
  in order to get the right results)

Signed-off-by: David Thompson <davthomp@redhat.com>
Extra strategy in MatchLocator to use DOM, and tweaks in various
Locators to properly handle DOM. Tests in JavaSearchTests help a lot
(currently ~130/270 passing)

It basically clones and adapt some methods and adapt them to DOM,
A new flag is introduced to decide the strategy.

Done-ish (often with remaining issues)
* FieldLocator
* LocalVariableLocator
* MethodLocator
* SuperTypeReferenceLocator
* TypeDeclarationLocator
* TypeParameterLocator
* TypeReferenceLocator
TODO:
* AndLocator
* ModuleLocator
* PackageDeclarationLocator
* PackageReferenceLocator
* OrLocator
* VariableLocator

Make some match locator work with DOM

Fix NPE, convert 4 errors to fails

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

Handle match files in the order they arrived

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

Fix testBug251827b and 5 others - import declarations java element must be found

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

Move the DOM-based search into the javac bundle

Signed-off-by: Rob Stryker <rob@oxbeef.net>

Cleanup

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

Move the DOM-based search into the javac bundle

Signed-off-by: Rob Stryker <stryker@redhat.com>
- precalculate extends or implements info
- When there is no prefix, suggest types declared in the same class,
  eg. suggest `MyInterface` in the following snippet

```java
public class MyClass implements | {

}

interface MyInterface {}
```

- Better recovery of bindings for classes with incomplete extends or
  implements sections
  - previously the binding key was always `*`, which caused many that
    relied on the binding key to break

- Use ExtendsOrImplements info filtering in more places

Signed-off-by: David Thompson <davthomp@redhat.com>
Make it capable of resolving *any* type visible to this build.
resolveWellKnownType method specifies a minimum set of supported types,
but the list is not meant to be exclusive and the definition of
"well-known" type is not so constraining.
So we allow test to pass even if a binding resolve can resolve
java.lang.Runnable as a well-known type.
+ do not add import for fully qualified types
Signed-off-by: Rob Stryker <stryker@redhat.com>
Signed-off-by: Rob Stryker <stryker@redhat.com>
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