alexeyinkin commented on code in PR #24588:
URL: https://github.com/apache/beam/pull/24588#discussion_r1044183582
##########
playground/frontend/playground_components/tools/extract_symbols_java/build.gradle:
##########
@@ -0,0 +1,34 @@
+plugins {
+ id 'java'
+}
+
+group 'org.example'
+version '1.0-SNAPSHOT'
+
+repositories {
+ mavenCentral()
+}
+
+ext {
+ javaMainClass = "com.playground.extract_symbols.Main"
+}
+
+dependencies {
+ implementation group: 'com.github.javaparser', name: 'javaparser-core',
version: '3.23.1'
+ testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.0'
+ testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.9.0'
+}
+
+test {
+ useJUnitPlatform()
+}
Review Comment:
Looks unused.
##########
playground/frontend/playground_components/build.gradle.kts:
##########
@@ -138,6 +138,7 @@ tasks.register("extractBeamSymbols") {
dependsOn("ensureSymbolsDirectoryExists")
dependsOn("extractBeamSymbolsGo")
dependsOn("extractBeamSymbolsPython")
+ dependsOn("tools:extract_symbols_java:buildJava")
Review Comment:
We need a unified naming here.
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
+ HashMap<String, ClassInfo> classInfoMap = collectClassInfo(sdkPath);
+ String yamlString = buildYamlString(classInfoMap);
+ System.out.println(yamlString);
+ }
+
+ private static HashMap<String, ClassInfo> collectClassInfo(String sdkPath)
throws IOException {
+ var classInfoList = new HashMap<String, ClassInfo>();
+ var paths = new File(sdkPath).toPath().toAbsolutePath();
+ Files.walk(paths).forEach(path -> {
+ var stringPath = path.toString();
+ if (stringPath.endsWith(".java") && !stringPath.contains("test")) {
+ var fileName =
stringPath.substring(stringPath.lastIndexOf("/") + 1).replace(".java", "");
+ try {
+ var unit = StaticJavaParser.parse(path);
+ if (unit.getClassByName(fileName).isPresent()) {
+ buildClass(classInfoList,
unit.getClassByName(fileName).get());
+ }
+ } catch (IOException | ParseProblemException ignored) {
+ }
+ }
+ });
+
+ return classInfoList;
+ }
+
+ private static void buildClass(HashMap<String, ClassInfo> classInfoList,
ClassOrInterfaceDeclaration cl) {
Review Comment:
If following naming conventions of the other SDK's extractors, this should
be `addClassSymbols`.
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
+ HashMap<String, ClassInfo> classInfoMap = collectClassInfo(sdkPath);
+ String yamlString = buildYamlString(classInfoMap);
+ System.out.println(yamlString);
+ }
+
+ private static HashMap<String, ClassInfo> collectClassInfo(String sdkPath)
throws IOException {
+ var classInfoList = new HashMap<String, ClassInfo>();
+ var paths = new File(sdkPath).toPath().toAbsolutePath();
+ Files.walk(paths).forEach(path -> {
+ var stringPath = path.toString();
+ if (stringPath.endsWith(".java") && !stringPath.contains("test")) {
+ var fileName =
stringPath.substring(stringPath.lastIndexOf("/") + 1).replace(".java", "");
+ try {
+ var unit = StaticJavaParser.parse(path);
+ if (unit.getClassByName(fileName).isPresent()) {
+ buildClass(classInfoList,
unit.getClassByName(fileName).get());
+ }
+ } catch (IOException | ParseProblemException ignored) {
+ }
+ }
+ });
+
+ return classInfoList;
+ }
+
+ private static void buildClass(HashMap<String, ClassInfo> classInfoList,
ClassOrInterfaceDeclaration cl) {
+ if (!cl.isPublic()) {
+ return;
+ }
+
+ ClassInfo classInfo;
+ if (classInfoList.containsKey(cl.getNameAsString())) {
+ classInfo = classInfoList.get(cl.getNameAsString());
+ } else {
+ classInfo = new ClassInfo();
+ classInfoList.put(cl.getNameAsString(), classInfo);
+ }
+
+ cl.findAll(MethodDeclaration.class).forEach(method -> {
+ if (method.isPublic()) {
+ classInfo.publicMethods.add(method.getNameAsString());
+ }
+ });
+ cl.findAll(FieldDeclaration.class).forEach(field -> {
+ if (field.isPublic()) {
+
classInfo.publicFields.add(field.getVariable(0).getNameAsString());
+ }
+ });
+ }
+
+ private static String buildYamlString(HashMap<String, ClassInfo>
classInfoList) {
+ final String[] resultList = new String[classInfoList.size()];
+ var i = 0;
+ for (Map.Entry<String, ClassInfo> entry : classInfoList.entrySet()) {
+ String name = entry.getKey();
+ ClassInfo classInfo = entry.getValue();
+ var yaml = toYaml(name, classInfo);
+ resultList[i] = yaml;
+ i++;
+ }
+ return String.join("\n", resultList);
+ }
+
+ private static String toYaml(String name, ClassInfo classInfo) {
+ var methods = classInfo.publicMethods;
+ var properties = classInfo.publicFields;
+
+ var methodsString = methods.isEmpty() ? "" : String.join("\n - ",
methods);
+ var propertiesString = properties.isEmpty() ? "" : String.join("\n -
", properties);
+
+ var methodsListString = methodsString.isEmpty() ? "" : " \n methods:
\n - " + methodsString;
+ var propertiesListString = propertiesString.isEmpty() ? "" : " \n
properties: \n - " + propertiesString;
+
+ return String.format("%s: %s%s", name, methodsListString,
propertiesListString);
+ }
Review Comment:
We should find an existing YAML writer. If we change the output format, such
custom writer is too hard to update.
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/ClassInfo.java:
##########
@@ -0,0 +1,9 @@
+package com.playground.extract_symbols;
+
+import java.util.HashSet;
+import java.util.Set;
+
+public class ClassInfo {
+ Set<String> publicMethods = new HashSet<>();
+ Set<String> publicFields = new HashSet<>();
Review Comment:
`final`?
##########
playground/frontend/playground_components/tools/extract_symbols_java/build.gradle:
##########
@@ -0,0 +1,34 @@
+plugins {
+ id 'java'
+}
+
+group 'org.example'
+version '1.0-SNAPSHOT'
Review Comment:
Delete these?
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
Review Comment:
Most of the `var`s can be `final` in this file.
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
+ HashMap<String, ClassInfo> classInfoMap = collectClassInfo(sdkPath);
+ String yamlString = buildYamlString(classInfoMap);
+ System.out.println(yamlString);
+ }
+
+ private static HashMap<String, ClassInfo> collectClassInfo(String sdkPath)
throws IOException {
+ var classInfoList = new HashMap<String, ClassInfo>();
Review Comment:
A map is not a *list* although in Python and Go we have poor naming too.
`ClassInfos` is a bit better while we search for the perfect name.
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
+ HashMap<String, ClassInfo> classInfoMap = collectClassInfo(sdkPath);
+ String yamlString = buildYamlString(classInfoMap);
+ System.out.println(yamlString);
+ }
+
+ private static HashMap<String, ClassInfo> collectClassInfo(String sdkPath)
throws IOException {
Review Comment:
If following naming conventions of the other SDK's extractors, this should
be `getDirSymbols`.
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
+ HashMap<String, ClassInfo> classInfoMap = collectClassInfo(sdkPath);
+ String yamlString = buildYamlString(classInfoMap);
+ System.out.println(yamlString);
+ }
+
+ private static HashMap<String, ClassInfo> collectClassInfo(String sdkPath)
throws IOException {
+ var classInfoList = new HashMap<String, ClassInfo>();
+ var paths = new File(sdkPath).toPath().toAbsolutePath();
+ Files.walk(paths).forEach(path -> {
+ var stringPath = path.toString();
+ if (stringPath.endsWith(".java") && !stringPath.contains("test")) {
Review Comment:
A check for containing 'test' is not the grea**test** one.
##########
playground/frontend/playground_components/tools/extract_symbols_java/build.gradle:
##########
@@ -0,0 +1,34 @@
+plugins {
Review Comment:
`./gradlew rat`
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
+ HashMap<String, ClassInfo> classInfoMap = collectClassInfo(sdkPath);
+ String yamlString = buildYamlString(classInfoMap);
+ System.out.println(yamlString);
+ }
+
+ private static HashMap<String, ClassInfo> collectClassInfo(String sdkPath)
throws IOException {
+ var classInfoList = new HashMap<String, ClassInfo>();
+ var paths = new File(sdkPath).toPath().toAbsolutePath();
Review Comment:
A `path` is single.
##########
playground/frontend/playground_components/tools/extract_symbols_java/build.gradle:
##########
@@ -0,0 +1,34 @@
+plugins {
+ id 'java'
+}
+
+group 'org.example'
+version '1.0-SNAPSHOT'
+
+repositories {
+ mavenCentral()
+}
+
+ext {
+ javaMainClass = "com.playground.extract_symbols.Main"
+}
+
+dependencies {
+ implementation group: 'com.github.javaparser', name: 'javaparser-core',
version: '3.23.1'
+ testImplementation 'org.junit.jupiter:junit-jupiter-api:5.9.0'
+ testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.9.0'
+}
+
+test {
+ useJUnitPlatform()
+}
+
+tasks.register("buildJava") {
+ doLast {
+ exec {
+ commandLine "java", "-classpath",
sourceSets.main.runtimeClasspath.getAsPath(), javaMainClass
+ args("../../../../../sdks/java")
Review Comment:
Why not follow this?
```
exec {
executable("python3")
args(
"tools/extract_symbols_python/extract_symbols_python.py",
"../../../sdks/python/apache_beam",
)
standardOutput =
FileOutputStream("playground/frontend/playground_components/assets/symbols/python.g.yaml")
}
```
##########
settings.gradle.kts:
##########
@@ -61,6 +61,8 @@ include(":playground")
include(":playground:backend")
include(":playground:frontend")
include(":playground:frontend:playground_components")
+include(":playground:frontend:playground_components:tools:extract_symbols_java")
+project(":playground:frontend:playground_components:tools:extract_symbols_java").projectDir
= file("playground/frontend/playground_components/tools/extract_symbols_java")
Review Comment:
Isn't this the default `projectDir`?
##########
playground/frontend/playground_components/tools/extract_symbols_java/build.gradle:
##########
@@ -0,0 +1,34 @@
+plugins {
Review Comment:
This looks like KTS in a file named to be in Groovy.
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
+ HashMap<String, ClassInfo> classInfoMap = collectClassInfo(sdkPath);
+ String yamlString = buildYamlString(classInfoMap);
+ System.out.println(yamlString);
+ }
+
+ private static HashMap<String, ClassInfo> collectClassInfo(String sdkPath)
throws IOException {
+ var classInfoList = new HashMap<String, ClassInfo>();
+ var paths = new File(sdkPath).toPath().toAbsolutePath();
+ Files.walk(paths).forEach(path -> {
+ var stringPath = path.toString();
+ if (stringPath.endsWith(".java") && !stringPath.contains("test")) {
Review Comment:
Please extract to something like
https://github.com/apache/beam/blob/58b4d46655d94374f3d3564752dc12eb98b95456/playground/frontend/playground_components/tools/extract_symbols_go/extract_symbols_go.go#L83
##########
playground/frontend/playground_components/tools/extract_symbols_java/settings.gradle:
##########
@@ -0,0 +1 @@
+rootProject.name = 'extract_java_symbols'
Review Comment:
`extract_symbols_java`?
##########
playground/frontend/playground_components/build.gradle.kts:
##########
@@ -178,4 +179,4 @@ tasks.register("extractBeamSymbolsPython") {
standardOutput =
FileOutputStream("playground/frontend/playground_components/assets/symbols/python.g.yaml")
}
}
-}
+}
Review Comment:
Check the newlines.
##########
playground/frontend/playground_components/tools/extract_symbols_java/src/main/java/com/playground/extract_symbols/Main.java:
##########
@@ -0,0 +1,93 @@
+package com.playground.extract_symbols;
+
+import com.github.javaparser.ParseProblemException;
+import com.github.javaparser.StaticJavaParser;
+import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
+import com.github.javaparser.ast.body.FieldDeclaration;
+import com.github.javaparser.ast.body.MethodDeclaration;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+
+public class Main {
+ public static void main(String[] args) throws IOException {
+ var sdkPath = args[0];
+ HashMap<String, ClassInfo> classInfoMap = collectClassInfo(sdkPath);
+ String yamlString = buildYamlString(classInfoMap);
+ System.out.println(yamlString);
+ }
+
+ private static HashMap<String, ClassInfo> collectClassInfo(String sdkPath)
throws IOException {
+ var classInfoList = new HashMap<String, ClassInfo>();
+ var paths = new File(sdkPath).toPath().toAbsolutePath();
+ Files.walk(paths).forEach(path -> {
+ var stringPath = path.toString();
+ if (stringPath.endsWith(".java") && !stringPath.contains("test")) {
+ var fileName =
stringPath.substring(stringPath.lastIndexOf("/") + 1).replace(".java", "");
Review Comment:
I guess `path` (whatever its type is) should have some API to extract a file
name without an extension.
--
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]
