<chore>[devtool]: add zstack-dev-tool static checker#3479
Open
zstack-robot-2 wants to merge 2 commits into5.5.12from
Open
<chore>[devtool]: add zstack-dev-tool static checker#3479zstack-robot-2 wants to merge 2 commits into5.5.12from
zstack-robot-2 wants to merge 2 commits into5.5.12from
Conversation
Resolves: ZSTAC-0 Change-Id: I94735b8ca0146f2ea8deed644f72def232fb24dc
Resolves: ZSTAC-0 - Fix SdkChecker/ApiHelperChecker passed() to actually detect failures instead of always returning true - Fix classIndex in ApiMessageScanner to also build FQCN index, preventing silent overwrites when two classes share a simple name - Fix DirectoryStream resource leak in DevTool.addPluginDirs() (use try-with-resources) - Auto-discover source modules instead of hardcoded list - Fix resolveTypeClass default branch (return simpleName, not java.lang.simpleName for unknown types) - Add ApiMessageScannerTest with 7 tests covering RestRequest parsing, abstract/NoSDK exclusion, param extraction, SdkChecker field mismatch detection, and ApiHelperChecker missing method detection Test: mvn package (15/15 tests pass) Change-Id: Id494379077c18bb49b2776e74e959afdd6dc1917 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
总览为ZStack开发工具添加了完整的CLI框架，包含Java源码扫描器、配置与API文档检查器、文档生成器等功能模块，支持验证和生成开发工具相关的文档与检查。 变化
序列图sequenceDiagram
participant User as 用户
participant CLI as DevTool CLI
participant Scanner as 扫描器
participant Checker as 检查器
participant Generator as 生成器
User->>CLI: 执行命令 (check/generate/scan)
CLI->>CLI: 检测项目根目录
alt Check命令
CLI->>Scanner: 扫描源目录
Scanner->>Scanner: 解析Java文件
Scanner-->>CLI: 返回元数据
CLI->>Checker: 验证配置/API
Checker->>Checker: 检查文档完整性
Checker-->>CLI: 返回检查结果
CLI-->>User: 输出检查报告
else Generate命令
CLI->>Scanner: 扫描源目录
Scanner-->>CLI: 返回元数据
CLI->>Generator: 生成文档
Generator->>Generator: 创建Markdown文件
Generator-->>CLI: 返回生成统计
CLI-->>User: 输出生成报告
else Scan命令
CLI->>Scanner: 扫描源目录
Scanner->>Scanner: 分析代码结构
Scanner-->>CLI: 返回扫描结果
CLI-->>User: 输出扫描详情
end
估计代码审查工作量🎯 4 (复杂) | ⏱️ ~60 分钟 诗
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
zstack-dev-tool/src/main/java/org/zstack/devtool/checker/ApiHelperChecker.java (1)
56-78:IOException处理后结果状态不一致当读取文件成功但在处理过程中发生
IOException时，result.totalMethods保持为 0，但missingMethods列表也为空，导致passed()返回true，这可能掩盖实际问题。建议在 catch 块中将所有消息标记为缺失，或记录错误状态以便调用者知晓检查未完成。
♻️ 建议的修复
} catch (IOException e) { System.out.println("[ApiHelper] ERROR - Cannot read " + apiHelperFile + ": " + e.getMessage()); + // Mark all as missing since we couldn't verify + for (ApiMessageInfo msg : messages) { + result.missingMethods.add(msg.getHelperMethodName() + " (from " + msg.getClassName() + ") - unverified"); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/src/main/java/org/zstack/devtool/checker/ApiHelperChecker.java` around lines 56 - 78, When an IOException occurs while reading ApiHelper.groovy in ApiHelperChecker, the code currently leaves result.totalMethods at 0 and result.missingMethods empty causing passed() to be true; update the catch block to mark the check as failed by setting result.totalMethods to messages.size() and adding every ApiMessageInfo.getHelperMethodName() (with its msg.getClassName()) to result.missingMethods, or alternatively set an explicit error flag on result to indicate the check did not complete so callers of ApiHelperChecker can detect the failure.zstack-dev-tool/src/main/java/org/zstack/devtool/model/GlobalConfigInfo.java (1)
69-71: 范围显示可读性可优化当只设置了
numberGreaterThan或numberLessThan其中一个时，会显示类似[5, 9223372036854775807]这样的范围，包含Long.MAX_VALUE或Long.MIN_VALUE，可读性较差。可以考虑使用更友好的表示方式，如
(>5)或[5, ∞)。♻️ 可选优化
if (numberGreaterThan != Long.MIN_VALUE || numberLessThan != Long.MAX_VALUE) { - return "[" + numberGreaterThan + ", " + numberLessThan + "]"; + String lower = numberGreaterThan == Long.MIN_VALUE ? "-∞" : String.valueOf(numberGreaterThan); + String upper = numberLessThan == Long.MAX_VALUE ? "∞" : String.valueOf(numberLessThan); + return "[" + lower + ", " + upper + "]"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/src/main/java/org/zstack/devtool/model/GlobalConfigInfo.java` around lines 69 - 71, The current range string in GlobalConfigInfo uses raw Long.MIN_VALUE/Long.MAX_VALUE when only one bound is set, producing unreadable outputs like "[5, 9223372036854775807]"; update the formatting logic that builds the range (the code using numberGreaterThan and numberLessThan) to detect the sentinel values (Long.MIN_VALUE and Long.MAX_VALUE) and emit human-friendly forms: if only numberGreaterThan is set output "(>X)" or "[X, ∞)"; if only numberLessThan is set output "(<Y)" or "(-∞, Y]" (choose consistent inclusive/exclusive notation used elsewhere), and keep the existing "[low, high]" when both bounds are set—use numberGreaterThan and numberLessThan checks rather than printing raw min/max.zstack-dev-tool/src/main/java/org/zstack/devtool/scanner/ApiMessageScanner.java (1)
181-192:hasApiNoSee设置逻辑存在冗余在第 186 行，如果
hasApiNoSee为true则执行continue，所以第 192 行的param.setNoSee(hasApiNoSee)永远只会设置为false。这段代码可以简化。♻️ 建议简化
boolean hasApiParam = field.getAnnotationByName("APIParam").isPresent(); - boolean hasApiNoSee = field.getAnnotationByName("APINoSee").isPresent(); // SDK only includes fields with `@APIParam` (excluding `@APINoSee`) if (!hasApiParam) continue; - if (hasApiNoSee) continue; + if (field.getAnnotationByName("APINoSee").isPresent()) continue; for (VariableDeclarator var : field.getVariables()) { ApiParamInfo param = new ApiParamInfo(); param.setFieldName(var.getNameAsString()); param.setFieldType(resolveFieldType(field)); - param.setNoSee(hasApiNoSee); + // noSee is always false here since we continued above if APINoSee present🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/src/main/java/org/zstack/devtool/scanner/ApiMessageScanner.java` around lines 181 - 192, The code computes hasApiNoSee but then continues when it's true, so param.setNoSee(hasApiNoSee) is always false and redundant; in ApiMessageScanner remove the needless param.setNoSee(hasApiNoSee) call (or replace it with an explicit param.setNoSee(false) if you prefer clarity) and keep the existing continue check for hasApiNoSee, ensuring hasApiNoSee is only used to skip fields and not to set the ApiParamInfo flag.zstack-dev-tool/src/test/java/org/zstack/devtool/GlobalConfigScannerTest.java (2)
15-15: 未使用的导入
java.util.Arrays被导入但未在代码中使用。🧹 建议移除
-import java.util.Arrays;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/src/test/java/org/zstack/devtool/GlobalConfigScannerTest.java` at line 15, The import java.util.Arrays in the GlobalConfigScannerTest class is unused—remove the unused import statement from GlobalConfigScannerTest (the import line for java.util.Arrays) to clean up the code and avoid IDE/compiler warnings; no other changes to class methods or tests are needed.
26-33: 辅助方法在同一测试中多次调用可能失败
createJavaFile()方法每次调用都尝试创建src/main/java目录结构。如果在同一个测试中多次调用，tempDir.newFolder()会因目录已存在而抛出异常。当前测试用例每个只调用一次，所以没有问题，但如果未来扩展测试可能会遇到问题。🔧 可选改进：使用 getRoot() 替代 newFolder()
private File createJavaFile(String filename, String content) throws Exception { - File dir = tempDir.newFolder("src", "main", "java"); + File dir = new File(tempDir.getRoot(), "src/main/java"); + dir.mkdirs(); File file = new File(dir, filename); try (FileWriter w = new FileWriter(file)) { w.write(content); } return dir; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/src/test/java/org/zstack/devtool/GlobalConfigScannerTest.java` around lines 26 - 33, createJavaFile currently calls tempDir.newFolder("src","main","java") which will fail if called multiple times; change createJavaFile to use tempDir.getRoot() (or tempDir.getRoot().toPath()) as the base, then ensure the "src/main/java" directories exist by creating them with mkdirs() or Files.createDirectories before writing the file; update the method to return the created File (the variable file) and keep the FileWriter logic in createJavaFile to write content to that file.zstack-dev-tool/src/main/java/org/zstack/devtool/checker/GlobalConfigDocChecker.java (1)
76-100: 可选优化：contains()检查可能产生误报当前使用简单的
content.contains()进行一致性检查，例如类型"Long"可能会错误匹配文档中的"LongRunning"等字符串。对于文档一致性检查来说，这种轻微的误报风险是可以接受的，但如果需要更精确的匹配，可以考虑使用正则表达式或检查特定的 Markdown 格式。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/src/main/java/org/zstack/devtool/checker/GlobalConfigDocChecker.java` around lines 76 - 100, The checkConsistency method in GlobalConfigDocChecker uses naive content.contains(...) calls (e.g., for expectedType, expectedDefault, and config.getName()) which can yield false positives like matching "Long" inside "LongRunning"; update checkConsistency to perform stricter matches by using regex word-boundary or Markdown-aware patterns (e.g., \bTYPE\b or matching code/heading tokens) when verifying expectedType, expectedDefault and name presence so only exact tokens or specific markdown constructs are considered; replace the content.contains(...) checks for expectedType, expectedDefault and config.getName() with these regex/markdown checks while preserving the same result.inconsistent messages when a mismatch is detected.zstack-dev-tool/src/main/java/org/zstack/devtool/DevTool.java (1)
243-274: 硬编码的排除目录列表可能需要维护
getSourceDirs()方法硬编码了要排除的目录列表（Line 252-256）。当项目结构变化或新增模块时，可能需要更新此列表。建议考虑：
- 将排除列表提取为常量以便维护
- 或者采用包含策略（检测含有
pom.xml的目录）而非排除策略这是一个可选的维护性改进，当前实现功能正确。
🔧 可选：提取排除列表为常量
+ private static final Set<String> EXCLUDED_DIRS = Set.of( + "premium", "plugin", "sdk", "test", "testlib", + "zstack-dev-tool", "build", "doc", "conf", "tools" + ); + private List<Path> getSourceDirs() { List<Path> dirs = new ArrayList<>(); try (DirectoryStream<Path> stream = Files.newDirectoryStream(projectRoot)) { for (Path child : stream) { if (!Files.isDirectory(child)) continue; String name = child.getFileName().toString(); - if (name.startsWith(".") || "premium".equals(name) || "plugin".equals(name) - || "sdk".equals(name) || "test".equals(name) || "testlib".equals(name) - || "zstack-dev-tool".equals(name) || "build".equals(name) - || "doc".equals(name) || "conf".equals(name) || "tools".equals(name)) { + if (name.startsWith(".") || EXCLUDED_DIRS.contains(name)) { continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/src/main/java/org/zstack/devtool/DevTool.java` around lines 243 - 274, getSourceDirs currently uses a hard-coded exclusion list inline which is brittle; extract that list to a single static constant (e.g., EXCLUDED_DIRS as a Set<String>) and replace the inline name.startsWith/equals checks with a membership check against EXCLUDED_DIRS in getSourceDirs (referencing getSourceDirs and projectRoot to locate the loop), or switch to an inclusion strategy by checking for module markers (e.g., child.resolve("pom.xml") or other canonical files) and only adding child.resolve("src/main/java") when that marker exists; update addPluginDirs usage unchanged and ensure the new constant is well-named and documented.zstack-dev-tool/dev-tool (2)
10-10: 可选：静默模式可能隐藏构建错误
mvn package -DskipTests -q的-q（quiet）模式会抑制大部分输出。如果构建失败，用户可能看不到有用的错误信息。可以考虑使用
-q仅在成功时生效，或者保留警告级别的输出。🔧 可选：保留警告输出
- cd "$SCRIPT_DIR" && mvn package -DskipTests -q + cd "$SCRIPT_DIR" && mvn package -DskipTests --batch-mode -Dorg.slf4j.simpleLogger.defaultLogLevel=WARN或者更简单地移除
-q：- cd "$SCRIPT_DIR" && mvn package -DskipTests -q + cd "$SCRIPT_DIR" && mvn package -DskipTests🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/dev-tool` at line 10, The current build command string `cd "$SCRIPT_DIR" && mvn package -DskipTests -q` uses Maven's `-q` which can hide useful error output; change it to either remove `-q` entirely or make it conditional: run `mvn package -DskipTests -q`, capture the exit code, and if it fails re-run `mvn package -DskipTests` (without `-q`) so you surface errors; update the script where that exact command appears to implement one of these two behaviors.
17-17: 可选：添加cd失败检查如果
cd "$SCRIPT_DIR/.."失败（虽然不太可能），脚本会在错误的目录下执行 Java 命令。🔧 可选：添加错误检查
-cd "$SCRIPT_DIR/.." && java -jar "$JAR" "$@" +cd "$SCRIPT_DIR/.." || { echo "ERROR: Cannot change to project root"; exit 1; } +java -jar "$JAR" "$@"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@zstack-dev-tool/dev-tool` at line 17, Ensure the script checks that changing directory succeeds before running Java: after the cd "$SCRIPT_DIR/.." step (the line currently using cd "$SCRIPT_DIR/.." && java -jar "$JAR" "$@"), detect a failed cd and abort with an error message and non‑zero exit (e.g., emit a clear error via echo or logger and exit 1) so the subsequent java -jar invocation never runs in the wrong directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@zstack-dev-tool/src/main/java/org/zstack/devtool/checker/SdkChecker.java`:
- Around line 57-63: The Files.list(sdkDir) stream is not closed, risking
resource leaks; update the block that sets result.totalSdkFiles to open the
stream in a try-with-resources (or otherwise ensure closure) when calling
Files.list(sdkDir). Specifically, wrap Files.list(sdkDir) in a try (Stream<Path>
stream = Files.list(sdkDir)) { result.totalSdkFiles = (int) stream.filter(p ->
p.getFileName().toString().endsWith("Action.java")).count(); } and preserve the
existing catch(IOException e) behavior that sets result.totalSdkFiles = 0;
reference the variables/methods result.totalSdkFiles, sdkDir, Files.list and the
lambda p -> p.getFileName().toString().endsWith("Action.java") when locating the
code to change.
- Around line 88-95: The Files.walk(...) Stream in SdkChecker (the method that
searches for actionName + ".java") is not closed; wrap the Files.walk(sdkDir)
call in a try-with-resources (e.g., try (Stream<Path> stream =
Files.walk(sdkDir)) { ... }) so the stream is closed automatically, then perform
the filter/findFirst on that stream and return found.orElse(null); also keep the
existing IOException catch to return null.
In
`@zstack-dev-tool/src/main/java/org/zstack/devtool/generator/GlobalConfigDocGenerator.java`:
- Line 89: In GlobalConfigDocGenerator (the code that builds the Markdown table
row using sb.append("|").append(res).append("\n")), fix the Markdown row by
appending the closing pipe before the newline; update the string building in
that method so each row becomes "|<value>|\n" (i.e., change the append sequence
to add the trailing "|" prior to "\n") to ensure proper Markdown table
rendering.
In `@zstack-dev-tool/src/main/java/org/zstack/devtool/model/ApiMessageInfo.java`:
- Around line 43-48: 在 getHelperMethodName() 中对从 getActionName() 返回的 action
做防御性检查：先检查 action 是否为 null 且长度是否至少为 6（因代码期望去掉尾部 "Action"），如果不满足则给出明确的处理（例如抛出
IllegalStateException/IllegalArgumentException 带上具体信息，或返回一个明确的默认值），否则按现有逻辑截取
name = action.substring(0, action.length() - 6) 并小写首字母后返回；确保引用
getActionName()、getHelperMethodName() 的位置都能接受你选择的错误处理方式并在异常消息中包含有助排查的上下文（比如
className 或 action）。
---
Nitpick comments:
In `@zstack-dev-tool/dev-tool`:
- Line 10: The current build command string `cd "$SCRIPT_DIR" && mvn package
-DskipTests -q` uses Maven's `-q` which can hide useful error output; change it
to either remove `-q` entirely or make it conditional: run `mvn package
-DskipTests -q`, capture the exit code, and if it fails re-run `mvn package
-DskipTests` (without `-q`) so you surface errors; update the script where that
exact command appears to implement one of these two behaviors.
- Line 17: Ensure the script checks that changing directory succeeds before
running Java: after the cd "$SCRIPT_DIR/.." step (the line currently using cd
"$SCRIPT_DIR/.." && java -jar "$JAR" "$@"), detect a failed cd and abort with an
error message and non‑zero exit (e.g., emit a clear error via echo or logger and
exit 1) so the subsequent java -jar invocation never runs in the wrong
directory.
In
`@zstack-dev-tool/src/main/java/org/zstack/devtool/checker/ApiHelperChecker.java`:
- Around line 56-78: When an IOException occurs while reading ApiHelper.groovy
in ApiHelperChecker, the code currently leaves result.totalMethods at 0 and
result.missingMethods empty causing passed() to be true; update the catch block
to mark the check as failed by setting result.totalMethods to messages.size()
and adding every ApiMessageInfo.getHelperMethodName() (with its
msg.getClassName()) to result.missingMethods, or alternatively set an explicit
error flag on result to indicate the check did not complete so callers of
ApiHelperChecker can detect the failure.
In
`@zstack-dev-tool/src/main/java/org/zstack/devtool/checker/GlobalConfigDocChecker.java`:
- Around line 76-100: The checkConsistency method in GlobalConfigDocChecker uses
naive content.contains(...) calls (e.g., for expectedType, expectedDefault, and
config.getName()) which can yield false positives like matching "Long" inside
"LongRunning"; update checkConsistency to perform stricter matches by using
regex word-boundary or Markdown-aware patterns (e.g., \bTYPE\b or matching
code/heading tokens) when verifying expectedType, expectedDefault and name
presence so only exact tokens or specific markdown constructs are considered;
replace the content.contains(...) checks for expectedType, expectedDefault and
config.getName() with these regex/markdown checks while preserving the same
result.inconsistent messages when a mismatch is detected.
In `@zstack-dev-tool/src/main/java/org/zstack/devtool/DevTool.java`:
- Around line 243-274: getSourceDirs currently uses a hard-coded exclusion list
inline which is brittle; extract that list to a single static constant (e.g.,
EXCLUDED_DIRS as a Set<String>) and replace the inline name.startsWith/equals
checks with a membership check against EXCLUDED_DIRS in getSourceDirs
(referencing getSourceDirs and projectRoot to locate the loop), or switch to an
inclusion strategy by checking for module markers (e.g.,
child.resolve("pom.xml") or other canonical files) and only adding
child.resolve("src/main/java") when that marker exists; update addPluginDirs
usage unchanged and ensure the new constant is well-named and documented.
In
`@zstack-dev-tool/src/main/java/org/zstack/devtool/model/GlobalConfigInfo.java`:
- Around line 69-71: The current range string in GlobalConfigInfo uses raw
Long.MIN_VALUE/Long.MAX_VALUE when only one bound is set, producing unreadable
outputs like "[5, 9223372036854775807]"; update the formatting logic that builds
the range (the code using numberGreaterThan and numberLessThan) to detect the
sentinel values (Long.MIN_VALUE and Long.MAX_VALUE) and emit human-friendly
forms: if only numberGreaterThan is set output "(>X)" or "[X, ∞)"; if only
numberLessThan is set output "(<Y)" or "(-∞, Y]" (choose consistent
inclusive/exclusive notation used elsewhere), and keep the existing "[low,
high]" when both bounds are set—use numberGreaterThan and numberLessThan checks
rather than printing raw min/max.
In
`@zstack-dev-tool/src/main/java/org/zstack/devtool/scanner/ApiMessageScanner.java`:
- Around line 181-192: The code computes hasApiNoSee but then continues when
it's true, so param.setNoSee(hasApiNoSee) is always false and redundant; in
ApiMessageScanner remove the needless param.setNoSee(hasApiNoSee) call (or
replace it with an explicit param.setNoSee(false) if you prefer clarity) and
keep the existing continue check for hasApiNoSee, ensuring hasApiNoSee is only
used to skip fields and not to set the ApiParamInfo flag.
In
`@zstack-dev-tool/src/test/java/org/zstack/devtool/GlobalConfigScannerTest.java`:
- Line 15: The import java.util.Arrays in the GlobalConfigScannerTest class is
unused—remove the unused import statement from GlobalConfigScannerTest (the
import line for java.util.Arrays) to clean up the code and avoid IDE/compiler
warnings; no other changes to class methods or tests are needed.
- Around line 26-33: createJavaFile currently calls
tempDir.newFolder("src","main","java") which will fail if called multiple times;
change createJavaFile to use tempDir.getRoot() (or tempDir.getRoot().toPath())
as the base, then ensure the "src/main/java" directories exist by creating them
with mkdirs() or Files.createDirectories before writing the file; update the
method to return the created File (the variable file) and keep the FileWriter
logic in createJavaFile to write content to that file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: a4e99274-7e43-47f7-9da7-ff60f514e20a
⛔ Files ignored due to path filters (1)
zstack-dev-tool/pom.xmlis excluded by!**/*.xml
📒 Files selected for processing (14)
zstack-dev-tool/.gitignorezstack-dev-tool/dev-toolzstack-dev-tool/src/main/java/org/zstack/devtool/DevTool.javazstack-dev-tool/src/main/java/org/zstack/devtool/checker/ApiHelperChecker.javazstack-dev-tool/src/main/java/org/zstack/devtool/checker/GlobalConfigDocChecker.javazstack-dev-tool/src/main/java/org/zstack/devtool/checker/SdkChecker.javazstack-dev-tool/src/main/java/org/zstack/devtool/generator/GlobalConfigDocGenerator.javazstack-dev-tool/src/main/java/org/zstack/devtool/model/ApiMessageInfo.javazstack-dev-tool/src/main/java/org/zstack/devtool/model/ApiParamInfo.javazstack-dev-tool/src/main/java/org/zstack/devtool/model/GlobalConfigInfo.javazstack-dev-tool/src/main/java/org/zstack/devtool/scanner/ApiMessageScanner.javazstack-dev-tool/src/main/java/org/zstack/devtool/scanner/GlobalConfigScanner.javazstack-dev-tool/src/test/java/org/zstack/devtool/ApiMessageScannerTest.javazstack-dev-tool/src/test/java/org/zstack/devtool/GlobalConfigScannerTest.java
Comment on lines
+57
to
+63
| try { | ||
| result.totalSdkFiles = (int) Files.list(sdkDir) | ||
| .filter(p -> p.getFileName().toString().endsWith("Action.java")) | ||
| .count(); | ||
| } catch (IOException e) { | ||
| result.totalSdkFiles = 0; | ||
| } |
There was a problem hiding this comment.
Files.list() 返回的 Stream 需要关闭以避免资源泄漏
Files.list() 返回一个需要显式关闭的 Stream<Path>。当前实现可能导致文件句柄泄漏。
🔧 建议使用 try-with-resources
// count existing SDK action files
- try {
- result.totalSdkFiles = (int) Files.list(sdkDir)
+ try (var stream = Files.list(sdkDir)) {
+ result.totalSdkFiles = (int) stream
.filter(p -> p.getFileName().toString().endsWith("Action.java"))
.count();
} catch (IOException e) {
result.totalSdkFiles = 0;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@zstack-dev-tool/src/main/java/org/zstack/devtool/checker/SdkChecker.java`
around lines 57 - 63, The Files.list(sdkDir) stream is not closed, risking
resource leaks; update the block that sets result.totalSdkFiles to open the
stream in a try-with-resources (or otherwise ensure closure) when calling
Files.list(sdkDir). Specifically, wrap Files.list(sdkDir) in a try (Stream<Path>
stream = Files.list(sdkDir)) { result.totalSdkFiles = (int) stream.filter(p ->
p.getFileName().toString().endsWith("Action.java")).count(); } and preserve the
existing catch(IOException e) behavior that sets result.totalSdkFiles = 0;
reference the variables/methods result.totalSdkFiles, sdkDir, Files.list and the
lambda p -> p.getFileName().toString().endsWith("Action.java") when locating the
code to change.
Comment on lines
+88
to
+95
| try { | ||
| Optional<Path> found = Files.walk(sdkDir) | ||
| .filter(p -> p.getFileName().toString().equals(actionName + ".java")) | ||
| .findFirst(); | ||
| return found.orElse(null); | ||
| } catch (IOException e) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Files.walk() 返回的 Stream 同样需要关闭
与 Files.list() 类似，Files.walk() 返回的 Stream 也需要关闭以释放文件系统资源。
🔧 建议使用 try-with-resources
// Check subdirectories (some actions are in sub-packages)
- try {
- Optional<Path> found = Files.walk(sdkDir)
+ try (var stream = Files.walk(sdkDir)) {
+ Optional<Path> found = stream
.filter(p -> p.getFileName().toString().equals(actionName + ".java"))
.findFirst();
return found.orElse(null);
} catch (IOException e) {
return null;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@zstack-dev-tool/src/main/java/org/zstack/devtool/checker/SdkChecker.java`
around lines 88 - 95, The Files.walk(...) Stream in SdkChecker (the method that
searches for actionName + ".java") is not closed; wrap the Files.walk(sdkDir)
call in a try-with-resources (e.g., try (Stream<Path> stream =
Files.walk(sdkDir)) { ... }) so the stream is closed automatically, then perform
the filter/findFirst on that stream and return found.orElse(null); also keep the
existing IOException catch to return null.
| if (resources != null && !resources.isEmpty()) { | ||
| sb.append("||\n|---|\n"); | ||
| for (String res : resources) { | ||
| sb.append("|").append(res).append("\n"); |
There was a problem hiding this comment.
Markdown 表格格式可能不完整
Line 89 的表格行缺少闭合的 |，当前输出格式为 |resource\n 而不是 |resource|\n，这可能导致某些 Markdown 解析器无法正确渲染表格。
🔧 建议修复
for (String res : resources) {
- sb.append("|").append(res).append("\n");
+ sb.append("| ").append(res).append(" |\n");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@zstack-dev-tool/src/main/java/org/zstack/devtool/generator/GlobalConfigDocGenerator.java`
at line 89, In GlobalConfigDocGenerator (the code that builds the Markdown table
row using sb.append("|").append(res).append("\n")), fix the Markdown row by
appending the closing pipe before the newline; update the string building in
that method so each row becomes "|<value>|\n" (i.e., change the append sequence
to add the trailing "|" prior to "\n") to ensure proper Markdown table
rendering.
Comment on lines
+43
to
+48
| public String getHelperMethodName() { | ||
| // CreateZoneAction -> createZone | ||
| String action = getActionName(); | ||
| String name = action.substring(0, action.length() - 6); // strip "Action" | ||
| return Character.toLowerCase(name.charAt(0)) + name.substring(1); | ||
| } |
There was a problem hiding this comment.
getHelperMethodName() 中缺少对空/短字符串的保护
如果 className 为 null，或者转换后的 action 名称少于 6 个字符，此方法会抛出 NullPointerException 或 StringIndexOutOfBoundsException。
🛡️ 建议添加防御性检查
public String getHelperMethodName() {
// CreateZoneAction -> createZone
String action = getActionName();
+ if (action == null || action.length() <= 6) {
+ return action != null ? action.toLowerCase() : "";
+ }
String name = action.substring(0, action.length() - 6); // strip "Action"
+ if (name.isEmpty()) {
+ return "";
+ }
return Character.toLowerCase(name.charAt(0)) + name.substring(1);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@zstack-dev-tool/src/main/java/org/zstack/devtool/model/ApiMessageInfo.java`
around lines 43 - 48, 在 getHelperMethodName() 中对从 getActionName() 返回的 action
做防御性检查：先检查 action 是否为 null 且长度是否至少为 6（因代码期望去掉尾部 "Action"），如果不满足则给出明确的处理（例如抛出
IllegalStateException/IllegalArgumentException 带上具体信息，或返回一个明确的默认值），否则按现有逻辑截取
name = action.substring(0, action.length() - 6) 并小写首字母后返回；确保引用
getActionName()、getHelperMethodName() 的位置都能接受你选择的错误处理方式并在异常消息中包含有助排查的上下文（比如
className 或 action）。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
zstack-dev-tool独立模块，基于 JavaParser 做静态源码分析./runMavenProfile sdk|apihelper|globalconfigdocmd的 CI 检查，无需编译，秒级完成v2 修复内容 (@@2)
passed()修复: 之前永远返回 true，现在正确检测 field mismatch 和 missing methodsjava.lang.前缀验证输出 (
dev-tool check allon upstream/5.5.12)测试
mvn package: 15/15 tests pass (8 GlobalConfig + 7 ApiMessage/SDK/ApiHelper)包含
scanner/: GlobalConfigScanner + ApiMessageScanner（AST 解析）checker/: GlobalConfigDocChecker + SdkChecker + ApiHelperCheckergenerator/: GlobalConfigDocGenerator（自动生成缺失的 markdown 文档）model/: GlobalConfigInfo + ApiMessageInfo + ApiParamInfodev-toolResolves: ZSTAC-0
sync from gitlab !9339