You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/08/31 08:15:08 UTC
[GitHub] [netbeans] sdedic commented on a change in pull request #2333: [NETBEANS-4745] Include immutable objects for fxml editor identification
sdedic commented on a change in pull request #2333:
URL: https://github.com/apache/netbeans/pull/2333#discussion_r479957948
##########
File path: javafx/javafx2.editor/src/org/netbeans/modules/javafx2/editor/completion/beans/BeanModelBuilder.java
##########
@@ -310,6 +310,78 @@ private void processGetters() {
addMapProperty(m, n);
}
}
+ if (allProperties.isEmpty() && !resultInfo.isFxInstance()) {
+ processGettersCheckForImmutables();
+ }
+ }
+
+ private static final String NAMED_ARG = "javafx.beans.NamedArg";
+
+ /** Some javafx classes, such as Insets, are immutable and do not have
+ * no argument constructors or setters; so they are not found.
+ * Accept a property if there is a getter with a corresponding
+ * constructor param declared with NamedArg annotation; use constructor
+ * with the most NamedArg parameters.
+ * <p/>
+ * One alternate strategy would be to provide a document with lines like:
+ * "Insets: top bottom left right" and use this info.
+ */
+ private void processGettersCheckForImmutables() {
+ Set<String> propsConstructor = Collections.emptySet();
+ Set<String> props1 = new HashSet<>();
+ CHECK_CONSTR: for (ExecutableElement c : ElementFilter.constructorsIn(classElement.getEnclosedElements())) {
+ props1.clear();
+
+ CHECK_PARAMS: for (VariableElement p : c.getParameters()) {
+ for (AnnotationMirror am : p.getAnnotationMirrors()) {
+ if (NAMED_ARG.equals(am.getAnnotationType().toString())) {
Review comment:
I would suggest to use either `((TypeElement)(am.getAnnotationType()).asElement()).getQualifiedName().toString()` or use `compilationInfo.getElements().getTypeElement(NAMED_ARG)` and compare `asElement()` to it. toString() is a diagnostic method, may change.
##########
File path: javafx/javafx2.editor/src/org/netbeans/modules/javafx2/editor/completion/beans/BeanModelBuilder.java
##########
@@ -310,6 +310,78 @@ private void processGetters() {
addMapProperty(m, n);
}
}
+ if (allProperties.isEmpty() && !resultInfo.isFxInstance()) {
+ processGettersCheckForImmutables();
+ }
+ }
+
+ private static final String NAMED_ARG = "javafx.beans.NamedArg";
+
+ /** Some javafx classes, such as Insets, are immutable and do not have
+ * no argument constructors or setters; so they are not found.
+ * Accept a property if there is a getter with a corresponding
+ * constructor param declared with NamedArg annotation; use constructor
+ * with the most NamedArg parameters.
+ * <p/>
+ * One alternate strategy would be to provide a document with lines like:
+ * "Insets: top bottom left right" and use this info.
+ */
+ private void processGettersCheckForImmutables() {
+ Set<String> propsConstructor = Collections.emptySet();
+ Set<String> props1 = new HashSet<>();
+ CHECK_CONSTR: for (ExecutableElement c : ElementFilter.constructorsIn(classElement.getEnclosedElements())) {
+ props1.clear();
+
+ CHECK_PARAMS: for (VariableElement p : c.getParameters()) {
+ for (AnnotationMirror am : p.getAnnotationMirrors()) {
+ if (NAMED_ARG.equals(am.getAnnotationType().toString())) {
+ String prop = am.getElementValues().values().iterator().next().toString();
Review comment:
Better check the Map.Entry's key for `"value"`, there's also a `defaultValue` declared. Also it may be better to use `getValue()` to get the String instead of stripping `"` from `toString()`.
##########
File path: javafx/javafx2.editor/src/org/netbeans/modules/javafx2/editor/completion/beans/FxDefinitionKind.java
##########
@@ -47,6 +47,11 @@
*/
LIST,
+ /**
+ * Readonly/immutable object, type is getter return type.
+ */
+ GETTER,
Review comment:
This value is not used anywhere after construction - correct ?
##########
File path: javafx/javafx2.editor/src/org/netbeans/modules/javafx2/editor/completion/beans/BeanModelBuilder.java
##########
@@ -310,6 +310,78 @@ private void processGetters() {
addMapProperty(m, n);
}
}
+ if (allProperties.isEmpty() && !resultInfo.isFxInstance()) {
+ processGettersCheckForImmutables();
+ }
+ }
+
+ private static final String NAMED_ARG = "javafx.beans.NamedArg";
+
+ /** Some javafx classes, such as Insets, are immutable and do not have
+ * no argument constructors or setters; so they are not found.
+ * Accept a property if there is a getter with a corresponding
+ * constructor param declared with NamedArg annotation; use constructor
+ * with the most NamedArg parameters.
+ * <p/>
+ * One alternate strategy would be to provide a document with lines like:
+ * "Insets: top bottom left right" and use this info.
+ */
+ private void processGettersCheckForImmutables() {
+ Set<String> propsConstructor = Collections.emptySet();
+ Set<String> props1 = new HashSet<>();
+ CHECK_CONSTR: for (ExecutableElement c : ElementFilter.constructorsIn(classElement.getEnclosedElements())) {
+ props1.clear();
+
+ CHECK_PARAMS: for (VariableElement p : c.getParameters()) {
+ for (AnnotationMirror am : p.getAnnotationMirrors()) {
+ if (NAMED_ARG.equals(am.getAnnotationType().toString())) {
+ String prop = am.getElementValues().values().iterator().next().toString();
+ // Strip off leading/trailing quotes
+ props1.add(prop.substring(1, prop.length() - 1));
+ continue CHECK_PARAMS;
+ }
+ }
+ // At least one of the parameters wasn't NAMED_ARG; skip it.
+ continue CHECK_CONSTR;
+ }
+ if (propsConstructor.size() < props1.size()) {
+ propsConstructor = new HashSet<>(props1);
+ }
+ }
+
+ if (propsConstructor.isEmpty()) {
+ return;
+ }
+
+ // problem if not all constructor args are covered?
+ boolean fxInstance = false;
+ for (ExecutableElement m : getters) {
+ String n = getPropertyName(m.getSimpleName().toString());
+ if (propsConstructor.contains(n)) {
+ addGetterOnlyProperty(m, n);
+ fxInstance = true;
+ }
+ }
+ resultInfo.setFxInstance(fxInstance);
+
+ // if (fxInstance) {
Review comment:
Leftover, or turn to Logger.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists