matthiasblaesing commented on code in PR #5444:
URL: https://github.com/apache/netbeans/pull/5444#discussion_r1099183945
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/editor/ContextUtilities.java:
##########
@@ -116,18 +116,22 @@ public static String
getAttributeTokenImage(DocumentContext context) {
* Returns the prefix from the element's tag.
*/
public static String getPrefixFromTag(String tagName) {
- if(tagName == null) return null;
- return (tagName.indexOf(":") == -1) ? null : // NOI18N
- tagName.substring(0, tagName.indexOf(":")); // NOI18N
+ if(tagName == null) {
+ return null;
+ }
+ return (tagName.indexOf(':') == -1) ? null : // NOI18N
+ tagName.substring(0, tagName.indexOf(':')); // NOI18N
Review Comment:
I suggest to replace this with a normal `if`. In my mind ternaries loose
their value if they spread over more than one line.
The NOI18N comments can be removed as you switched from `string` to `char`.
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/editor/DocumentContext.java:
##########
@@ -214,10 +209,7 @@ public boolean equals(Object obj) {
return false;
}
final DocumentContext other = (DocumentContext) obj;
- if (this.document != other.document && (this.document == null ||
!this.document.equals(other.document))) {
- return false;
- }
- return true;
+ return !(this.document != other.document && (this.document == null ||
!this.document.equals(other.document)));
Review Comment:
This can be simpified to `! Objects.equals(this.document, other.document)`
(at least if my mind parses that correctly, please also check).
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/editor/completion/JPACodeCompletionProvider.java:
##########
@@ -146,7 +146,7 @@ protected void query(CompletionResultSet resultSet,
Document doc, int caretOffse
if (anchorOffset > -1) {
resultSet.setAnchorOffset(anchorOffset);
}
- } catch (Exception e) {
+ } catch (MissingResourceException | ParseException e) {
Review Comment:
I would also catch `RuntimeException`
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/editor/hyperlink/NamedQueryHyperlinkProvider.java:
##########
@@ -155,73 +152,69 @@ private void goToNQ(Document doc, int offset) {
if (ent != null) {
try {
- js.runUserActionTask(new Task<CompilationController>() {
-
- @Override
- public void run(CompilationController parameter) throws
Exception {
- parameter.toPhase(JavaSource.Phase.RESOLVED);
- AnnotationMirror foundAm = null;
- AnnotationValue get = null;
- Trees trees = parameter.getTrees();
-
- TypeElement entityElement =
parameter.getElements().getTypeElement(entClasst);trees.getSourcePositions().getStartPosition(parameter.getCompilationUnit(),
trees.getPath(entityElement).getLeaf());
- List<? extends AnnotationMirror> annotationMirrors =
entityElement.getAnnotationMirrors();
- if (annotationMirrors != null) {
- Iterator<? extends AnnotationMirror> iterator =
annotationMirrors.iterator();
- while (iterator.hasNext() && foundAm == null) {
- AnnotationMirror next = iterator.next();
- if
(next.getAnnotationType().toString().equals("javax.persistence.NamedQueries"))
{//NOI18N
-
- Map<? extends ExecutableElement, ? extends
AnnotationValue> maps = next.getElementValues();
-
- for (AnnotationValue vl : maps.values()) {
- List lst = (List) vl.getValue();
- for (Object val : lst) {
- if (val instanceof
AnnotationMirror) {
- AnnotationMirror am =
(AnnotationMirror) val;
- if
("javax.persistence.NamedQuery".equals(am.getAnnotationType().toString()))
{//NOI18N
- Map<? extends
ExecutableElement, ? extends AnnotationValue> elementValues =
am.getElementValues();
- for (ExecutableElement el
: elementValues.keySet()) {
- if
(el.getSimpleName().contentEquals("name")) { //NOI18N
- get =
elementValues.get(el);
- if
(get.getValue().toString().equals(nam)) {
- foundAm = am;
- break;
- }
+ js.runUserActionTask( (CompilationController parameter) -> {
+ parameter.toPhase(JavaSource.Phase.RESOLVED);
+ AnnotationMirror foundAm = null;
+ AnnotationValue get = null;
+ Trees trees = parameter.getTrees();
+
+ TypeElement entityElement =
parameter.getElements().getTypeElement(entClasst);trees.getSourcePositions().getStartPosition(parameter.getCompilationUnit(),
trees.getPath(entityElement).getLeaf());
Review Comment:
This looks fishy. Are there multiple statemetns on this line and is the
second a noop?
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/editor/completion/PUCompletor.java:
##########
@@ -208,34 +206,24 @@ public List<JPACompletionItem> doCompletion(final
CompletionContext context) {
private void doJavaCompletion(final FileObject fo, final JavaSource
js, final List<JPACompletionItem> results,
final String typedPrefix, final int substitutionOffset) throws
IOException {
- js.runUserActionTask(new Task<CompilationController>() {
-
- @Override
- public void run(CompilationController cc) throws Exception {
- cc.toPhase(Phase.ELEMENTS_RESOLVED);
- Project project = FileOwnerQuery.getOwner(fo);
- EntityClassScopeProvider provider =
project.getLookup().lookup(EntityClassScopeProvider.class);
- EntityClassScope ecs = null;
- Entity[] entities = null;
- if (provider != null) {
- ecs = provider.findEntityClassScope(fo);
- }
- if (ecs != null) {
- entities =
ecs.getEntityMappingsModel(false).runReadAction(new
MetadataModelAction<EntityMappingsMetadata, Entity[]>() {
-
- @Override
- public Entity[] run(EntityMappingsMetadata
metadata) throws Exception {
- return metadata.getRoot().getEntity();
- }
- });
- }
- // add classes
- if(entities != null) {
- for (Entity entity : entities) {
- if (typedPrefix.length() == 0 ||
entity.getClass2().toLowerCase().startsWith(typedPrefix.toLowerCase()) ||
entity.getName().toLowerCase().startsWith(typedPrefix.toLowerCase())) {
- JPACompletionItem item =
JPACompletionItem.createAttribValueItem(substitutionOffset, entity.getClass2());
- results.add(item);
- }
+ js.runUserActionTask( (CompilationController cc) -> {
+ cc.toPhase(Phase.ELEMENTS_RESOLVED);
+ Project project = FileOwnerQuery.getOwner(fo);
+ EntityClassScopeProvider provider =
project.getLookup().lookup(EntityClassScopeProvider.class);
+ EntityClassScope ecs = null;
+ Entity[] entities = null;
+ if (provider != null) {
+ ecs = provider.findEntityClassScope(fo);
+ }
+ if (ecs != null) {
+ entities =
ecs.getEntityMappingsModel(false).runReadAction( (EntityMappingsMetadata
metadata) -> metadata.getRoot().getEntity() );
+ }
+ // add classes
+ if(entities != null) {
+ for (Entity entity : entities) {
+ if (typedPrefix.length() == 0 ||
entity.getClass2().toLowerCase().startsWith(typedPrefix.toLowerCase()) ||
entity.getName().toLowerCase().startsWith(typedPrefix.toLowerCase())) {
Review Comment:
might be worth wrapping at the `||`
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/spi/jpql/support/JPAAttribute.java:
##########
@@ -143,55 +143,69 @@ public Class<?> getClass1() {
}
private void buildType(){
- TypeMirror tm = null;
- VariableElement var = Utils.getField(parent.getTypeElement(),
name);
- if(var == null){
- ExecutableElement acc =
Utils.getAccesor(parent.getTypeElement(), name);
- if(acc != null){
- tm = acc.getReturnType();
- }
- } else {
- tm = var.asType();
+ TypeMirror tm = null;
+ VariableElement var = Utils.getField(parent.getTypeElement(), name);
+ if(var == null){
+ ExecutableElement acc = Utils.getAccesor(parent.getTypeElement(),
name);
+ if(acc != null){
+ tm = acc.getReturnType();
}
- if( tm!=null ){
- if(tm.getKind() == TypeKind.DECLARED){
- DeclaredType declaredType = (DeclaredType) tm;
- if(declaredType.getTypeArguments()!=null &&
declaredType.getTypeArguments().size()>0) {//it's some generic type
- if(mType == IMappingType.ONE_TO_MANY || mType ==
IMappingType.MANY_TO_MANY) {//we suppose it should be for relationship mapping
only
- tm = declaredType.getTypeArguments().get(0);
- if(tm.getKind() == TypeKind.DECLARED){
- declaredType = (DeclaredType) tm;
+ } else {
+ tm = var.asType();
+ }
+ if( tm != null ){
+ if(null != tm.getKind()) {
Review Comment:
Would it make sense to pull this up into the upper if? To me these if ->
switch changes don't improve readabiliy, but ok.
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/unit/PUDataObject.java:
##########
@@ -174,21 +174,15 @@ public boolean parseDocument() {
} else if (isModified()){//if it's isn't modified and persistenc
eexits (parsed) no need to reparse
try{
String oldVersion = persistence.getVersion();
- java.io.InputStream is = getEditorSupport().getInputStream();
String version=Persistence.VERSION_1_0;
- try {
+ try (java.io.InputStream is =
getEditorSupport().getInputStream()) {
Review Comment:
Could you import `java.io.InputStream`?
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/wizard/EntityClosure.java:
##########
@@ -425,19 +399,9 @@ public boolean isModelReady() {
void waitModelIsReady(){
try {
- Future result = model.runReadActionWhenReady(new
MetadataModelAction<EntityMappingsMetadata, Boolean>() {
-
- @Override
- public Boolean run(EntityMappingsMetadata metadata) throws
Exception {
- return true;
- }
- });
+ Future result = model.runReadActionWhenReady(
(EntityMappingsMetadata metadata) -> true );
result.get();
- } catch (InterruptedException ex) {
- Exceptions.printStackTrace(ex);
- } catch (ExecutionException ex) {
- Exceptions.printStackTrace(ex);
- } catch (MetadataModelException ex) {
+ } catch (InterruptedException | ExecutionException |
MetadataModelException ex) {
Exceptions.printStackTrace(ex);
} catch (IOException ex) {
Review Comment:
Could be integrated into multi-catch.
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/wizard/fromdb/EntityClassesPanel.java:
##########
@@ -711,7 +703,7 @@ public boolean isValid() {
}
String packageName = getComponent().getPackageName();
- if (packageName.trim().equals("")) { // NOI18N
+ if (packageName.trim().isEmpty()) { // NOI18N
Review Comment:
Drop `NOI18N`
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/editor/completion/db/DBCompletionContextResolver.java:
##########
@@ -661,9 +685,13 @@ private List
completeManyToMany(JPACodeCompletionProvider.Context ctx, CCParser.
element:
for(VariableElement f : resultFields) {
for(javax.lang.model.element.Modifier
mod:f.getModifiers()){
-
if(javax.lang.model.element.Modifier.TRANSIENT.equals(mod))continue element;
+
if(javax.lang.model.element.Modifier.TRANSIENT.equals(mod)) {
+ continue element;
+ }
+ }
+
if(JpaControllerUtil.isAnnotatedWith(f,"javax.persistence.Transient")) {
+ continue;//NOI18N
Review Comment:
The NO18N comment belongs on the previous line (it marks strings, that must
not be translated)
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/editor/ContextUtilities.java:
##########
@@ -116,18 +116,22 @@ public static String
getAttributeTokenImage(DocumentContext context) {
* Returns the prefix from the element's tag.
*/
public static String getPrefixFromTag(String tagName) {
- if(tagName == null) return null;
- return (tagName.indexOf(":") == -1) ? null : // NOI18N
- tagName.substring(0, tagName.indexOf(":")); // NOI18N
+ if(tagName == null) {
+ return null;
+ }
+ return (tagName.indexOf(':') == -1) ? null : // NOI18N
+ tagName.substring(0, tagName.indexOf(':')); // NOI18N
}
/**
* Returns the local name from the element's tag.
*/
public static String getLocalNameFromTag(String tagName) {
- if(tagName == null) return null;
- return (tagName.indexOf(":") == -1) ? tagName : // NOI18N
- tagName.substring(tagName.indexOf(":")+1, tagName.length()); //
NOI18N
+ if(tagName == null) {
+ return null;
+ }
+ return (tagName.indexOf(':') == -1) ? tagName : // NOI18N
+ tagName.substring(tagName.indexOf(':')+1, tagName.length()); //
NOI18N
Review Comment:
The second argument to `substring` can be dropped. It default to the end of
the string.
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/editor/completion/PUCompletor.java:
##########
@@ -208,34 +206,24 @@ public List<JPACompletionItem> doCompletion(final
CompletionContext context) {
private void doJavaCompletion(final FileObject fo, final JavaSource
js, final List<JPACompletionItem> results,
final String typedPrefix, final int substitutionOffset) throws
IOException {
- js.runUserActionTask(new Task<CompilationController>() {
-
- @Override
- public void run(CompilationController cc) throws Exception {
- cc.toPhase(Phase.ELEMENTS_RESOLVED);
- Project project = FileOwnerQuery.getOwner(fo);
- EntityClassScopeProvider provider =
project.getLookup().lookup(EntityClassScopeProvider.class);
- EntityClassScope ecs = null;
- Entity[] entities = null;
- if (provider != null) {
- ecs = provider.findEntityClassScope(fo);
- }
- if (ecs != null) {
- entities =
ecs.getEntityMappingsModel(false).runReadAction(new
MetadataModelAction<EntityMappingsMetadata, Entity[]>() {
-
- @Override
- public Entity[] run(EntityMappingsMetadata
metadata) throws Exception {
- return metadata.getRoot().getEntity();
- }
- });
- }
- // add classes
- if(entities != null) {
- for (Entity entity : entities) {
- if (typedPrefix.length() == 0 ||
entity.getClass2().toLowerCase().startsWith(typedPrefix.toLowerCase()) ||
entity.getName().toLowerCase().startsWith(typedPrefix.toLowerCase())) {
- JPACompletionItem item =
JPACompletionItem.createAttribValueItem(substitutionOffset, entity.getClass2());
- results.add(item);
- }
+ js.runUserActionTask( (CompilationController cc) -> {
+ cc.toPhase(Phase.ELEMENTS_RESOLVED);
+ Project project = FileOwnerQuery.getOwner(fo);
+ EntityClassScopeProvider provider =
project.getLookup().lookup(EntityClassScopeProvider.class);
+ EntityClassScope ecs = null;
+ Entity[] entities = null;
+ if (provider != null) {
+ ecs = provider.findEntityClassScope(fo);
+ }
+ if (ecs != null) {
+ entities =
ecs.getEntityMappingsModel(false).runReadAction( (EntityMappingsMetadata
metadata) -> metadata.getRoot().getEntity() );
Review Comment:
The seems long - would it be equally clear if `EntityMappingsMetadata` is
dropped?
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/entitygenerator/SQLType.java:
##########
@@ -145,8 +148,8 @@ private Class getClassForCharType (Integer length, boolean
isNullable) {
// the no-arg getMemberType method are sufficient
private Class getClassForNumericType(Integer precision,
Integer scale, boolean isNullable) {
- int precValue = ((precision == null) ? -1 : precision.intValue());
- int scaleValue = ((scale == null) ? -1 : scale.intValue());
+ int precValue = ((precision == null) ? -1 : precision);
+ int scaleValue = ((scale == null) ? -1 : scale);
Review Comment:
Please drop the wrapping braces.
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/provider/ProviderUtil.java:
##########
@@ -161,8 +162,8 @@ private static Provider getContainerManagedProvider(Project
project) {
* @param pu the persistence unit whose database connection is to
* be retrieved; must not be null.
*
- * @return the connection specified in the given persistence unit or
- * <code>null</code> if it didn't specify a connectioh.
+ * @rerturn the connection specified in the given persistence unit or
Review Comment:
Typo introduced.
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/provider/ProviderUtil.java:
##########
@@ -206,14 +207,14 @@ public static DatabaseConnection
getConnection(PersistenceUnit pu) {
return null;
}
/**
- * Gets the database connection properties (irl,name,password) specified
in the given persistence
+ * Gets the database connection properties (url,name,password) specified
in the given persistence
* unit.
*
* @param pu the persistence unit whose database connection is to
* be retrieved; must not be null.
*
- * @return the connection properties specified in the given persistence
unit or
- * <code>null</code> if it didn't specify a connectioh.
+ * @rerturn the connection properties specified in the given persistence
unit or
Review Comment:
Typo introduced
##########
java/j2ee.persistence/src/org/netbeans/modules/j2ee/persistence/wizard/fromdb/SelectedTables.java:
##########
@@ -196,7 +196,9 @@ public String getClassName(Table table) {
String exClassName =
persistenceGen.getFQClassName(table.getName());
if(exClassName != null) {
int i = exClassName.lastIndexOf('.');
- if(i>-1)exClassName = exClassName.substring(i+1);
+ if(i>-1) {
Review Comment:
Formatting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
