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

Upgrade the JavaParser dependency #9

Merged
merged 1 commit into from Dec 19, 2017
Merged

Upgrade the JavaParser dependency #9

merged 1 commit into from Dec 19, 2017

Conversation

matozoid
Copy link
Contributor

@matozoid matozoid commented Nov 8, 2017

There are a lot of known bugs in JavaParser 2.4.0, making this a rather urgent upgrade.

String annotationPackageName = annotation
.findCompilationUnit()
.flatMap(CompilationUnit::getPackageDeclaration)
.flatMap(pkg -> Optional.of(pkg.getNameAsString())).orElse("???");
Copy link
Contributor Author

@matozoid matozoid Nov 8, 2017

Choose a reason for hiding this comment

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

Original code got a null when there is no package declaration. Not sure if that was the intention - it's ??? now.

Copy link
Owner

Choose a reason for hiding this comment

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

if there is no package declaration, then the annotation should declared in the default package right? What if we use an empty string as default value in the orElse?

methodDao.save(new Method(annotationMemberDeclaration.getName(),
isPublic(annotationMemberModifiers), isStatic(annotationMemberModifiers), isFinal(annotationMemberModifiers), isAbstract(annotationMemberModifiers), false, typeId));
methodDao.save(new Method(annotationMemberDeclaration.getNameAsString(),
annotationMemberDeclaration.isPublic(), false, false, annotationMemberDeclaration.isAbstract(), false, typeId));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

annotation interface members can't be static or final.

Copy link
Owner

Choose a reason for hiding this comment

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

indeed

List<Parameter> parameters = constructorDeclaration.getParameters();
String name = constructorDeclaration.getName();
String name = constructorDeclaration.getNameAsString();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Names are now nodes instead of strings, so there's a helper method to get the string value.

Copy link
Owner

Choose a reason for hiding this comment

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

ok, got it

@fmbenhassine
Copy link
Owner

Hi Danny,

Happy to see you here! Thank you for this PR and thank you for the wonderful java parser library!

I didn't have time to update this repo yet, it is still in PoC stage and the code was written in a quick and dirty manner. I was just trying to see if the idea works and if the relational model is good enough (see my blog post about it).

Anyway, I will merge your changes asap.

Best regards
Mahmoud

@matozoid
Copy link
Contributor Author

matozoid commented Nov 8, 2017

Cool! Just think of it like this: at least no potential user will be disappointed when it fails on parse errors on valid source code :-)

(Actually, it would be best to completely disable AST validations then... hmmmm...)

@fmbenhassine fmbenhassine merged commit 164616d into fmbenhassine:master Dec 19, 2017
@fmbenhassine
Copy link
Owner

I have some time to work on jql this week.

Thank you for your PR!

@matozoid matozoid deleted the upgrade-javaparser branch December 28, 2017 13:02
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.

None yet

2 participants