From 4cb0696df0ada910d90139cbbdc1525cba882d4d Mon Sep 17 00:00:00 2001 From: shenchucheng Date: Thu, 27 Jun 2024 16:01:16 +0800 Subject: [PATCH] add cr tool --- examples/cr.py | 15 ++ metagpt/ext/cr/__init__.py | 3 + metagpt/ext/cr/actions/code_review.py | 196 ++++++++++++++++++ metagpt/ext/cr/actions/modify_code.py | 114 +++++++++++ metagpt/ext/cr/points.json | 275 ++++++++++++++++++++++++++ metagpt/ext/cr/utils/__init__.py | 0 metagpt/ext/cr/utils/cleaner.py | 68 +++++++ metagpt/ext/cr/utils/schema.py | 20 ++ metagpt/tools/libs/cr.py | 90 +++++++++ 9 files changed, 781 insertions(+) create mode 100644 examples/cr.py create mode 100644 metagpt/ext/cr/__init__.py create mode 100644 metagpt/ext/cr/actions/code_review.py create mode 100644 metagpt/ext/cr/actions/modify_code.py create mode 100644 metagpt/ext/cr/points.json create mode 100644 metagpt/ext/cr/utils/__init__.py create mode 100644 metagpt/ext/cr/utils/cleaner.py create mode 100644 metagpt/ext/cr/utils/schema.py create mode 100644 metagpt/tools/libs/cr.py diff --git a/examples/cr.py b/examples/cr.py new file mode 100644 index 000000000..7171fc383 --- /dev/null +++ b/examples/cr.py @@ -0,0 +1,15 @@ +import fire + +from metagpt.roles.di.engineer2 import Engineer2 +from metagpt.tools.libs.cr import CodeReview + + +async def main(msg): + role = Engineer2(tools=["Plan", "Editor:write,read", "RoleZero", "ReviewAndRewriteCode", "CodeReview"]) + cr = CodeReview() + role.tool_execution_map.update({"CodeReview.review": cr.review, "CodeReview.fix": cr.fix}) + await role.run(msg) + + +if __name__ == "__main__": + fire.Fire(main) diff --git a/metagpt/ext/cr/__init__.py b/metagpt/ext/cr/__init__.py new file mode 100644 index 000000000..2bcf8efd0 --- /dev/null +++ b/metagpt/ext/cr/__init__.py @@ -0,0 +1,3 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# @Desc : diff --git a/metagpt/ext/cr/actions/code_review.py b/metagpt/ext/cr/actions/code_review.py new file mode 100644 index 000000000..70b85ff06 --- /dev/null +++ b/metagpt/ext/cr/actions/code_review.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# @Desc : + +import json +import re + +from unidiff import PatchedFile, PatchSet + +from metagpt.actions.action import Action +from metagpt.ext.cr.utils.cleaner import ( + add_line_num_on_patch, + get_code_block_from_patch, + rm_patch_useless_part, +) +from metagpt.ext.cr.utils.schema import Point +from metagpt.logs import logger +from metagpt.utils.common import parse_json_code_block + +CODE_REVIEW_PROMPT_TEMPLATE = """ +NOTICE +With the given pull-request(PR) Patch, and referenced Points(Code Standards), you should compare each point with the code one-by-one. + +The Patch code has added line number at the first character each line for reading, but the review should focus on new added code inside the `Patch` (lines starting with line number and '+'). +Each point is start with a line number and follows with the point description. + +## Patch +``` +{patch} +``` + +## Points +{points} + +## Output Format +```json +[ + {{ + "commented_file": "The file path which you give a comment from the patch", + "comment": "The chinese comment of code which do not meet point description and give modify suggestions", + "code_start_line": "the code start line number like `10` in the Patch of current comment,", + "code_end_line": "the code end line number like `15` in the Patch of current comment", + "point_id": "The point id which the `comment` references to" + }} +] +``` + +CodeReview guidelines: +- Generate code `comment` that do not meet the point description. +- Each `comment` should be restricted inside the `commented_file` +- Try to provide diverse and insightful comments across different `commented_file`. +- Don't suggest to add docstring unless it's necessary indeed. +- If the same code error occurs multiple times, it cannot be omitted, and all places need to be identified.But Don't duplicate at the same place with the same comment! +- Every line of code in the patch needs to be carefully checked, and laziness cannot be omitted. It is necessary to find out all the places. + +Just print the PR Patch comments in json format like **Output Format**. +""" + +CODE_REVIEW_COMFIRM_SYSTEM_PROMPT = """ +You are a professional engineer with Java stack, and good at code review comment result judgement. +""" + +CODE_REVIEW_COMFIRM_TEMPLATE = """ +## Code +``` +{code} +``` +## Code Review Comments +{comment} + +## Description of Defects +{desc} + +## Reference Example for Judgment +{example} + +## Your Task: +1. First, check if the code meets the requirements and does not violate any defects. If it meets the requirements and does not violate any defects, print `False` and do not proceed with further judgment. +2. If the check in step 1 does not print `False`, proceed to further judgment. Based on the "Reference Example for Judgment" provided, determine if the "Code" and "Code Review Comments" match. If they match, print `True`; otherwise, print `False`. + +Note: Your output should only be `True` or `False` without any explanations. +""" + + +class CodeReview(Action): + name: str = "CodeReview" + + def format_comments(self, comments: list[dict], points: list[Point], patch: PatchSet): + new_comments = [] + logger.debug(f"original comments: {comments}") + for cmt in comments: + for p in points: + if int(cmt.get("point_id", -1)) == p.id: + code_start_line = cmt.get("code_start_line") + code_end_line = cmt.get("code_end_line") + code = get_code_block_from_patch(patch, code_start_line, code_end_line) + + new_comments.append( + { + "commented_file": cmt.get("commented_file"), + "code": code, + "code_start_line": code_start_line, + "code_end_line": code_end_line, + "comment": cmt.get("comment"), + "point_id": p.id, + "point": p.text, + "point_detail": p.detail, + } + ) + break + + logger.debug(f"new_comments: {new_comments}") + return new_comments + + async def confirm_comments(self, patch: PatchSet, comments: list[dict], points: list[Point]) -> list[dict]: + points_dict = {point.id: point for point in points} + new_comments = [] + for cmt in comments: + point = points_dict[cmt.get("point_id")] + + code_start_line = cmt.get("code_start_line") + code_end_line = cmt.get("code_end_line") + # 如果代码位置为空的话，那么就将这条记录丢弃掉 + if not code_start_line or not code_end_line: + logger.info("False") + continue + + # 代码增加上下文，提升confirm的准确率 + code = get_code_block_from_patch(patch, str(max(1, int(code_start_line) - 3)), str(int(code_end_line) + 3)) + pattern = r"^[ \t\n\r(){}[\];,]*$" + if re.match(pattern, code): + code = get_code_block_from_patch( + patch, str(max(1, int(code_start_line) - 5)), str(int(code_end_line) + 5) + ) + prompt = CODE_REVIEW_COMFIRM_TEMPLATE.format( + code=code, + comment=cmt.get("comment"), + desc=point.text, + example=point.yes_example + "\n" + point.no_example, + ) + resp = await self.llm.aask(prompt, system_msgs=[CODE_REVIEW_COMFIRM_SYSTEM_PROMPT]) + if "True" in resp or "true" in resp: + new_comments.append(cmt) + logger.info(f"original comments num: {len(comments)}, confirmed comments num: {len(new_comments)}") + return new_comments + + async def cr_by_full_points(self, patch: PatchSet, points: list[Point]): + comments = [] + points_str = "\n".join([f"{p.id} {p.text}" for p in points]) + for patched_file in patch: + if patched_file.path.endswith(".py"): + points_str = "\n".join([f"{p.id} {p.text}" for p in points if p.language == "python"]) + elif patched_file.path.endswith(".java"): + points_str = "\n".join([f"{p.id} {p.text}" for p in points if p.language == "java"]) + else: + continue + if len(str(patched_file).splitlines()) >= 50: + cr_by_segment_points_comments = await self.cr_by_segment_points( + patched_file=patched_file, points=points + ) + comments += cr_by_segment_points_comments + continue + prompt = CODE_REVIEW_PROMPT_TEMPLATE.format(patch=str(patched_file), points=points_str) + resp = await self.llm.aask(prompt) + json_str = parse_json_code_block(resp)[0] + comments += json.loads(json_str) + + return comments + + async def cr_by_segment_points(self, patched_file: PatchedFile, points: list[Point]): + comments = [] + group_points = [points[i : i + 3] for i in range(0, len(points), 3)] + for group_point in group_points: + points_str = "\n".join([f"{p.id} {p.text}" for p in group_point]) + prompt = CODE_REVIEW_PROMPT_TEMPLATE.format(patch=str(patched_file), points=points_str) + resp = await self.llm.aask(prompt) + json_str = parse_json_code_block(resp)[0] + comments_batch = json.loads(json_str) + comments += comments_batch + + return comments + + async def run(self, patch: PatchSet, points: list[Point]): + patch: PatchSet = rm_patch_useless_part(patch) + patch: PatchSet = add_line_num_on_patch(patch) + + result = [] + comments = await self.cr_by_full_points(patch=patch, points=points) + if len(comments) != 0: + comments = self.format_comments(comments, points, patch) + comments = await self.confirm_comments(patch=patch, comments=comments, points=points) + for comment in comments: + if comment["code"]: + if not (comment["code"].startswith("-") or comment["code"].isspace()): + result.append(comment) + return result diff --git a/metagpt/ext/cr/actions/modify_code.py b/metagpt/ext/cr/actions/modify_code.py new file mode 100644 index 000000000..ad68710a7 --- /dev/null +++ b/metagpt/ext/cr/actions/modify_code.py @@ -0,0 +1,114 @@ +import datetime +import itertools +import re +from pathlib import Path +from typing import Optional + +from unidiff import PatchSet + +from metagpt.actions.action import Action +from metagpt.const import DEFAULT_WORKSPACE_ROOT +from metagpt.ext.cr.utils.cleaner import ( + add_line_num_on_patch, + get_code_block_from_patch, + rm_patch_useless_part, +) +from metagpt.utils.common import CodeParser +from metagpt.utils.report import EditorReporter + +SYSTEM_MSGS_PROMPT = """ +You're an adaptive software developer who excels at refining code based on user inputs. You're proficient in creating Git patches to represent code modifications. +""" + +MODIFY_CODE_PROMPT = """ +NOTICE +With the given pull-request(PR) Patch, and referenced Comments(Code Standards), you should modify the code according the Comments. + +The Patch code has added line no at the first character each line for reading, but the modification should focus on new added code inside the `Patch` (lines starting with line no and '+'). + +## Patch +``` +{patch} +``` + +## Comments +{comments} + +## Output Format + + + +Code Modification guidelines: +- Look at `point_detail`, modify the code by `point_detail`, use `code_start_line` and `code_end_line` to locate the problematic code, fix the problematic code by `point_detail` in Comments.Strictly,must handle the fix plan given by `point_detail` in every comment. +- Create a patch that satifies the git patch standard and your fixes need to be marked with '+' and '-',but notice:don't change the hunk header! +- Do not print line no in the new patch code. + +Just print the Patch in the format like **Output Format**. +""" + + +class ModifyCode(Action): + name: str = "Modify Code" + pr: str + + async def run(self, patch: PatchSet, comments: list[dict], output_dir: Optional[str] = None) -> str: + patch: PatchSet = rm_patch_useless_part(patch) + patch: PatchSet = add_line_num_on_patch(patch) + + # + for comment in comments: + code_start_line = comment.get("code_start_line") + code_end_line = comment.get("code_end_line") + # 如果代码位置为空的话，那么就将这条记录丢弃掉 + if code_start_line and code_end_line: + code = get_code_block_from_patch( + patch, str(max(1, int(code_start_line) - 3)), str(int(code_end_line) + 3) + ) + pattern = r"^[ \t\n\r(){}[\];,]*$" + if re.match(pattern, code): + code = get_code_block_from_patch( + patch, str(max(1, int(code_start_line) - 5)), str(int(code_end_line) + 5) + ) + # 代码增加上下文，提升代码修复的准确率 + comment["code"] = code + if comment["point_id"] in [24, 25, 26]: + comment["point_detail"] = comment["point_detail"] + "\n" + comment["comment"] + # 去掉CR时LLM给的comment的影响，应该使用既定的修复方案 + comment.pop("comment") + + # 按照 commented_file 字段进行分组 + comments.sort(key=lambda x: x["commented_file"]) + grouped_comments = { + key: list(group) for key, group in itertools.groupby(comments, key=lambda x: x["commented_file"]) + } + resp = None + for patched_file in patch: + patch_target_file_name = str(patched_file.target_file).split("/", maxsplit=1)[-1] + if patch_target_file_name not in grouped_comments: + continue + comments_prompt = "" + index = 1 + for grouped_comment in grouped_comments[patch_target_file_name]: + comments_prompt += f""" + + {grouped_comment} + \n + """ + prompt = MODIFY_CODE_PROMPT.format(patch=patched_file, comments=comments_prompt) + output_dir = ( + Path(output_dir) + if output_dir + else DEFAULT_WORKSPACE_ROOT / "modify_code" / str(datetime.date.today()) / self.pr + ) + patch_file = output_dir / f"{patch_target_file_name}.patch" + patch_file.parent.mkdir(exist_ok=True, parents=True) + async with EditorReporter(enable_llm_stream=True) as reporter: + await reporter.async_report( + {"type": "Patch", "src_path": str(patch_file), "filename": patch_file.name}, "meta" + ) + resp = await self.llm.aask(msg=prompt, system_msgs=[SYSTEM_MSGS_PROMPT]) + resp = CodeParser.parse_code(resp, "diff") + with open(patch_file, "w", encoding="utf-8") as file: + file.writelines(resp) + await reporter.async_report(patch_file) + return resp diff --git a/metagpt/ext/cr/points.json b/metagpt/ext/cr/points.json new file mode 100644 index 000000000..49b90e268 --- /dev/null +++ b/metagpt/ext/cr/points.json @@ -0,0 +1,275 @@ +[ + { + "id": 2, + "text": "避免未使用的临时变量", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：避免未使用的临时变量；对应Fixer：UnusedLocalVariableFixer；修复方案：删除未使用的临时变量", + "yes_example": "### 被判定为\"避免未使用的临时变量\"的例子\n<例子1>\npublic String initCreationForm(Map model) {\n\t\tOwner owner = new Owner();\n\t\tmodel.put(\"owner\", owner);\n\t\tint unusedVar = 10;\n\t\treturn VIEWS_OWNER_CREATE_OR_UPDATE_FORM;\n\t}\n上述代码中unusedVar变量未被使用，所以这个被判定为\"避免未使用的临时变量\"\n\n<例子2>\nint unusedVariable = 10;\nSystem.out.println(\"Hello, World!\");\n这段代码的变量\"unusedVariable\"未被使用或者引用，所以这个不能判定为\"避免未使用的临时变量\"\n", + "no_example": "### 不能被判定为\"避免未使用的临时变量\"的例子\n<例子1>\npublic void setTransientVariablesLocal(Map transientVariables) {\nthrow new UnsupportedOperationException(\"No execution active, no variables can be set\");\n}\n这段代码的\"transientVariables\"是函数参数而不是临时变量，虽然transientVariables没有被使用或者引用，但是这个也不能判定为\"避免未使用的临时变量\"\n\n\n<例子2>\npublic class TriggerCmd extends NeedsActiveExecutionCmd {\n protected Map transientVariables;\n public TriggerCmd(Map transientVariables) {\n this.transientVariables = transientVariables;\n }\n}\n上述代码中transientVariables不属于临时变量，它是类属性，且它在构造函数中被使用，所以这个不能被判定为\"避免未使用的临时变量\"\n" + }, + { + "id": 3, + "text": "不要使用 System.out.println 去打印", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：不要使用 System.out.println 去打印；对应Fixer：SystemPrintlnFixer；修复方案：注释System.out.println代码", + "yes_example": "### 被判定为\"不要使用 System.out.println 去打印\"的例子\n<例子1>\nSystem.out.println(\"Initializing new owner form.\");\n上述代码使用了\"System.out.println\"进行打印，所以这个被判定为\"不要使用 System.out.println 去打印\"\n", + "no_example": "### 不能被判定为\"不要使用 System.out.println 去打印\"的例子\n<例子1>\nthrow new IllegalStateException(\"There is no authenticated user, we need a user authenticated to find tasks\");\n上述代码是抛出异常的代码，没有使用\"System.out.print\"，所以这个不能被判定为\"不要使用 System.out.println 去打印\"\n" + }, + { + "id": 4, + "text": "避免函数中未使用的形参", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：避免函数中未使用的形参；修复方案：忽略", + "yes_example": "### 被判定为\"避免函数中未使用的形参\"的例子\n<例子1>\npublic void setTransientVariablesLocal(Map transientVariables) {\n throw new UnsupportedOperationException(\"No execution active, no variables can be set\");\n}这段代码中的形参\"transientVariables\"未在函数体内出现，所以这个被判定为\"避免函数中未使用的形参\"\n\n\n<例子2>\nprotected void modifyFetchPersistencePackageRequest(PersistencePackageRequest ppr, Map pathVars) {}\n这段代码中的形参\"ppr\"和\"pathVars\"未在函数体内出现，所以这个被判定为\"避免函数中未使用的形参\"\n", + "no_example": "### 不能被判定为\"避免函数中未使用的形参\"的例子\n<例子1>\npublic String processFindForm(@RequestParam(value = \"pageNo\", defaultValue = \"1\") int pageNo) {\n\tlastName = owner.getLastName();\n\treturn addPaginationModel(pageNo, paginationModel, lastName, ownersResults);\n}上述代码中的形参\"pageNo\"在当前函数'processFindForm'内被'return addPaginationModel(pageNo, paginationModel, lastName, ownersResults);'这一句被使用，虽然pageNo没有被用于逻辑计算，但作为了函数调用其他函数的参数使用了，所以这个不能被判定为\"避免函数中未使用的形参\"\n" + }, + { + "id": 5, + "text": "if语句块不能为空", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：if 语句块不能为空；对应Fixer：EmptyIfStmtFixer；修复方案：删除if语句块 或 适当的逻辑处理 或 注释说明为何为空", + "yes_example": "### 被判定为\"if语句块不能为空\"的例子\n<例子1>\npublic void emptyIfStatement() {\n\tif (getSpecialties().isEmpty()) {\n\t}\n}这段代码中的if语句块内容是空的，所以这个被判定为\"if语句块不能为空\"\n\n\n<例子2>\npublic void judgePersion() {\n\tif (persion != null) {\n\t\t// judge persion if not null\n\t}\n}\n这段代码中的if语句块虽然有内容，但是\"// judge persion if not null\"只是代码注释，if语句块内并没有实际的逻辑代码，所以这个被判定为\"if语句块不能为空\"\n", + "no_example": "### 不能被判定为\"if语句块不能为空\"的例子\n<例子1>\npublic void judgePersion() {\n\tif (persion != null) {\n\t\treturn 0;\n\t}\n}这段代码中的if语句块里有内容，且里面有非注释代码的逻辑代码\"return 0;\"，所以这个不能被判定为\"if语句块不能为空\"\n" + }, + { + "id": 6, + "text": "循环体不能为空", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：循环体不能为空；对应Fixer：EmptyStatementNotInLoopFixer；修复方案：删除对应while、for、foreach 循环体 或 添加适当的逻辑处理或者注释说明为何为空", + "yes_example": "### 被判定为\"循环体不能为空\"的例子\n<例子1>\npublic void emptyLoopBody() {\n\tfor (Specialty specialty : getSpecialties()) {\n\t}\n}这段代码中的for循环体的内容是空的，所以这个被判定为\"循环体不能为空\"\n\n\n<例子2>\npublic void emptyLoopBody() {\n\twhile (True) {\n\t\t// this is a code example\n\t}\n}这段代码中的while循环体的内容虽然不是空的，但内容只是代码注释，无逻辑内容，所以这个被判定为\"循环体不能为空\"\n\n\n<例子3>\npublic void emptyLoopBody() {\n\twhile (True) {\n\t\t\n\t}\n}这段代码中的while循环体内容是空的，所以这个被判定为\"循环体不能为空\"\n", + "no_example": "### 不能被判定为\"循环体不能为空\"的例子\n<例子1>\npublic void emptyLoopBody() {\n\tfor (Specialty specialty : getSpecialties()) {\n\t\ta = 1;\n\t\tif (a == 1) {\n\t\t\tretrun a;\n\t\t}\n\t}\n}上述代码的for循环体的内容不为空，且内容不全是代码注释，所以这个不能被判定为\"循环体不能为空\"\n" + }, + { + "id": 7, + "text": "避免使用 printStackTrace()，应该使用日志的方式去记录", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：避免使用 printStackTrace()，应该使 用日志的方式去记录；修复方案：用日志的方式去记录", + "yes_example": "### 被判定为\"避免使用 printStackTrace()，应该使用日志的方式去记录\"的例子\n<例子1>\npublic void usePrintStackTrace() {\n\ttry {\n\t\tthrow new Exception(\"Fake exception\");\n\t} catch (Exception e) {\n\t\te.printStackTrace();\n\t}\n}这段代码中的catch语句中使用了printStackTrace()，所以这个被判定为\"避免使用 printStackTrace()，应该使用日志的方式去记录\"\n", + "no_example": "### 不能被判定为\"避免使用 printStackTrace()，应该使用日志的方式去记录\"的例子\n<例子1>\npublic void usePrintStackTrace() {\n\ttry {\n\t\tthrow new Exception(\"Fake exception\");\n\t} catch (Exception e) {\n\t\tlogging.info(\"info\");\n\t}\n}这段代码的catch语句中使用的是日志记录的方式，所以这个不能被判定为\"避免使用 printStackTrace()，应该使用日志的方式去记录\"\n" + }, + { + "id": 8, + "text": "catch 语句块不能为空", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：catch 语句块不能为空；对应Fixer：EmptyCatchBlockFixer；修复方案：在catch里面添加注释", + "yes_example": "### 被判定为\"catch语句块不能为空\"的例子\n<例子1>\ntry {\n int[] array = new int[5];\n int number = array[10];\n} catch (ArrayIndexOutOfBoundsException e) {\n \n}\n这段代码中的catch语句中没有内容，所以这个被判定为\"catch语句块不能为空\"\n\n\n<例子2>\ntry {\n String str = null;\n str.length();\n} catch (NullPointerException e) {\n \n}这段代码中的catch语句中没有内容，所以这个被判定为\"catch语句块不能为空\"\n\n\n<例子3>\npublic class EmptyCatchExample {\n public static void main(String[] args) {\n try {\n // 尝试除以零引发异常\n int result = 10 / 0;\n } catch (ArithmeticException e) {\n \n }\n }\n}这段代码中的catch语句中没有内容，所以这个被判定为\"catch语句块不能为空\"\n\n<例子4>\ntry {\n FileReader file = new FileReader(\"nonexistentfile.txt\");\n} catch (FileNotFoundException e) {\n \n}这段代码中的catch语句中没有内容，所以这个被判定为\"catch语句块不能为空\"\n\n<例子5>\ntry {\n Object obj = \"string\";\n Integer num = (Integer) obj;\n} catch (ClassCastException e) {\n\t\n}这段代码中的catch语句中没有内容，所以这个被判定为\"catch语句块不能为空\"\n", + "no_example": "### 不能被判定为\"catch语句块不能为空\"的例子\n<例子1>\npersionNum = 1\ntry {\n\treturn True;\n} catch (Exception e) {\n\t// 如果人数为1则返回false\n\tif (persionNum == 1){\n\t\treturn False;\n\t}\n}这段代码的catch语句中不为空，所以不能把这个被判定为\"catch语句块不能为空\"\n\n\n<例子2>\ntry {\n\tthrow new Exception(\"Fake exception\");\n} catch (Exception e) {\n\te.printStackTrace();\n}这段代码的catch语句中虽然只有\"e.printStackTrace();\"但确实不为空，所以不能把这个被判定为\"catch语句块不能为空\"\n" + }, + { + "id": 9, + "text": "避免不必要的永真/永假判断", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：避免不必要的永真/永假判断；对应Fixer：UnconditionalIfStatement Fixer；修复方案：删除永真/永假判断逻辑", + "yes_example": "### 被判定为\"避免不必要的永真/永假判断\"的例子\n<例子1>\npublic void someMethod() {\n\twhile (true) {\n\t}\n}这段代码中的\"while (true)\"是一个使用true做判断条件，但是没有循环结束标记，所以这个被判定为\"避免不必要的永真/永假判断\"\n\n\n<例子2>\nif (true) {\n\tSystem.out.println(\"This is always true\");\n}这段代码中的\"if (true)\"是一个使用true条件做条件，但是没有循环结束标记，所以这个被判定为\"避免不必要的永真/永假判断\"\n\n\n<例子3>\na = 1;\nwhile(a > 0){\n\ta = a + 1\n}这段代码初始化a=1，是大于0的，while循环体的逻辑是每次加1，那么判断条件a > 0会永远是真的，不会退出循环，所以这个被判定为\"避免不必要的永真/永假判断\"\n<例子3>", + "no_example": "### 不能被判定为\"避免不必要的永真/永假判断\"的例子\n<例子1>\na = 0;\nwhile (a < 5) {\n\ta = a + 1;\n}这段代码中的a<5是一个判断，当执行了5次while语句中的逻辑a=a+1之后，a会满足a < 5，就会退出循环，所以这个能被判定为\"避免不必要的永真/永假判断\"\n" + }, + { + "id": 10, + "text": "switch 中 default 必须放在最后", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：switch 中 default 必须放在最后；对应Fixer：DefaultLabelNotLastInSwitchStmtFixer；修复方案：switch 中 default 放在最后", + "yes_example": "### 被判定为\"switch 中 default 必须放在最后\"的例子\n<例子1>\nswitch (number) {\n\tdefault:\n\t\tSystem.out.println(\"This is the default block, which is incorrectly placed here.\");\n\t\tbreak;\n\tcase 1:\n\t\tSystem.out.println(\"Number one\");\n\t\tbreak;\n\tcase 2:\n\t\tSystem.out.println(\"Number two\");\n\t\tbreak;\n}这段代码是一个switch语句，但是里面的default没有放在最后，所以这个被判定为\"switch 中 default 必须放在最后\"\n", + "no_example": "### 不能被判定为\"switch 中 default 必须放在最后\"的例子\n<例子1>\nswitch (number) {\ncase 3:\n\tSystem.out.println(\"Number one\");\n\tbreak;\ncase 4:\n\tSystem.out.println(\"Number two\");\n\tbreak;\ndefault:\n\tSystem.out.println(\"This is the default block, which is incorrectly placed here.\");\n\tbreak;\n}这段代码是一个switch语句且里面的default放在了最后，所以这个不能被判定为\"switch 中 default 必须放在最后\"\n" + }, + { + "id": 11, + "text": "未使用equals()函数对 String 作比较", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：未使用equals()函数对 String 作比较；对应Fixer：UnSynStaticDateFormatter Fixer；修复方案：使用equals()函数对 String 作比较", + "yes_example": "### 被判定为\"未使用equals()函数对 String 作比较\"的例子\n<例子1>\nif (existingPet != null && existingPet.getName() == petName) {\n\tresult.rejectValue(\"name\", \"duplicate\", \"already exists\");\n}这段代码中所涉及的existingPet.getName()和petName均是字符串，但是在if语句里做比较的时候使用了==而没有使用equals()对string做比较，所以这个被判定为\"未使用equals()函数对 String 作比较\"\n\n\n<例子2>\nString isOk = \"ok\";\nif (\"ok\" == isOk) {\n\tresult.rejectValue(\"name\", \"duplicate\", \"already exists\");\n}这段代码中的isOk是个字符串，但在if判断中与\"ok\"比较的时候使用的是==，未使用equals()对string做比较，所以这个被判定为\"未使用equals()函数对 String 作比较\"\n", + "no_example": "### 不能被判定为\"未使用equals()函数对 String 作比较\"的例子\n<例子1>\nif (PROPERTY_VALUE_YES.equalsIgnoreCase(readWriteReqNode))\n formProperty.setRequired(true);\n这段代码中的PROPERTY_VALUE_YES和readWriteReqNode均是字符串，在if语句里比较PROPERTY_VALUE_YES和readWriteReqNode的使用的是equalsIgnoreCase（字符串比较忽略大小写），所以equalsIgnoreCase也是符合使用equals()函数对 String 作比较的，所以这个不能被判定为\"未使用equals()函数对 String 作比较\"\n\n\n<例子2>\nString isOk = \"ok\";\nif (\"ok\".equals(isOk)) {\n\tresult.rejectValue(\"name\", \"duplicate\", \"already exists\");\n}这段代码中的isOk是个字符串，在if判断中与\"ok\"比较的时候使用的是equals()对string做比较，所以这个不能被判定为\"未使用equals()函数对 String 作比较\"\n" + }, + { + "id": 12, + "text": "禁止在日志中直接使用字符串输出异常，请使用占位符传递异常对象", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：禁止在日志中直接使用字符串输出异常，请使用占位符传递异常对象 输出异常；对应Fixer：ConcatExceptionFixer；修复方案：使用占位符传递异常对象", + "yes_example": "### 被判定为\"禁止在日志中直接使用字符串输出异常，请使用占位符传递异常对象\"的例子\n<例子1>\ntry {\n listenersNode = objectMapper.readTree(listenersNode.asText());\n} catch (Exception e) {\n LOGGER.info(\"Listeners node can not be read\", e);\n}这段代码中日志输出内容内容是直接使用字符串\"Listeners node can not be read\"拼接，日志输出异常时，应使用占位符输出异常信息，而不是直接使用字符串拼接，所以这个被判定为\"禁止在日志中直接使用字符串输出异常，请使用占位符传递异常对象\"\n", + "no_example": "### 不能被判定为\"禁止在日志中直接使用字符串输出异常，请使用占位符传递异常对象\"的例子\n<例子1>\nPersion persion = persionService.getPersion(1);\nif (persion == null){\n\tLOGGER.error(PERSION_NOT_EXIT);\n}这段代码中的PERSION_NOT_EXIT是一个用户自定义的异常常量，代表persion不存在，没有直接使用字符串\"persion not exit\"拼接，所以这个不能被判定为\"禁止在日志中直接使用字符串输出异常，请使用占位符传递异常对象\"\n<例子1>\n\n<例子2>\ntry {\n a = a + 1;\n} catch (Exception e) {\n Persion persion = persionService.getPersion(1);\n LOGGER.info(persion);\n}这段代码中输出日志没有直接使用字符串拼接，而是使用的Persion对象输出，所以这个不能被判定为\"禁止在日志中直接使用字符串输出异常，请使用占位符传递异常对象\"\n" + }, + { + "id": 13, + "text": "finally 语句块不能为空", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：finally 语句块不能为空；对应Fixer：EmptyFinallyBlockFixer；修复方案：删除空 finally 语句块", + "yes_example": "### 被判定为\"finally 语句块不能为空\"的例子\n<例子1>\ntry {\n\tPersion persion = persionService.getPersion(1);\n\treturn persion;\n} finally {\n\t\n}这段代码中的finally语句块内没有内容，所以这个被判定为\"finally 语句块不能为空\"\n\n\n<例子2>\ntry {\n\tSystem.out.println(\"Inside try block\");\n} finally {\n\t// 空的finally块，没有任何语句，这是一个缺陷\n}这段代码中的finally语句块内没有内容，所以这个被判定为\"finally 语句块不能为空\"\n\n\n<例子3>\ntry {\n int result = 10 / 0;\n} catch (ArithmeticException e) {\n e.printStackTrace();\n} finally {\n \n}这段代码中的finally语句块内没有内容，所以这个被判定为\"finally 语句块不能为空\"\n\n\n<例子4>\ntry {\n String str = null;\n System.out.println(str.length());\n} catch (NullPointerException e) {\n e.printStackTrace();\n} finally {\n \n}这段代码中的finally语句块内没有内容，所以这个被判定为\"finally 语句块不能为空\"\n\n\n<例子5>\ntry {\n int[] array = new int[5];\n int number = array[10];\n} catch (ArrayIndexOutOfBoundsException e) {\n e.printStackTrace();\n} finally {\n // 只有注释的 finally 语句块\n // 这是一个空的 finally 块\n}这段代码中的finally语句块内没有内容，所以这个被判定为\"finally 语句块不能为空\"\n\n\n<例子6>\ntry {\n FileReader file = new FileReader(\"nonexistentfile.txt\");\n} catch (FileNotFoundException e) {\n e.printStackTrace();\n} finally {\n // 只有空行的 finally 语句块\n \n}这段代码中的finally语句块内没有内容，所以这个被判定为\"finally 语句块不能为空\"\n", + "no_example": "### 不能被判定为\"finally 语句块不能为空\"的例子\n<例子1>\npublic void getPersion() {\n\ttry {\n\t\tPersion persion = persionService.getPersion(1);\n\t\tif (persion != null){ \n\t\t\treturn persion;\n\t\t}\n\t} finally {\n\t\treturn null;\n\t}\n}这段代码中的finally语句块中有非注释意外的内容\"return null;\"，所以这个不能被判定为\"finally 语句块不能为空\"\n" + }, + { + "id": 14, + "text": "try 语句块不能为空", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：try 语句块不能为空；对应Fixer：EmptyTryBlockFixer；修复方案：删除整个 try 语句", + "yes_example": "### 被判定为\"try 语句块不能为空\"的例子\n<例子1>\npublic void getPersion() {\n\ttry {\n\n\t}\n\treturn null;\n}这段代码中的try语句块内没有内容，所以这个被判定为\"try 语句块不能为空\"\n\n\n<例子2>\npublic void demoFinallyBlock() {\n\ttry {\n\n\t} finally {\n\t\treturn null;\n\t}\n}这段代码中的try语句块内没有内容，所以这个被判定为\"try 语句块不能为空\"\n\n\n<例子3>\ntry {\n \n} catch (Exception e) {\n e.printStackTrace();\n}这段代码中的try语句块内没有内容，所以这个被判定为\"try 语句块不能为空\"\n\n\n<例子4>\ntry {\n // 只有注释的 try 语句块\n\t\n} catch (Exception e) {\n e.printStackTrace();\n}这段代码中的try语句块内只有注释和空行，也可以认定为这种情况是try语句块内没有内容，所以这个被判定为\"try 语句块不能为空\"\n", + "no_example": "### 不能被判定为\"try 语句块不能为空\"的例子\n<例子1>\ntry {\n\ta = a + 1;\n} catch (Exception e) {\n\te.printStackTrace();\n}\n这段代码中的try语句块中有非注释意外的内容\"return null;\"，所以这个不能被判定为\"try 语句块不能为空\"\n" + }, + { + "id": 15, + "text": "避免对象进行不必要的 NULL或者null 检查", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：避免对象进行不必要的 NULL或者null 检查；对应Fixer：LogicalOpNpeFixer；修复方案：删除对对象不必要的 NULL 检查的逻辑", + "yes_example": "### 被判定为\"避免对象进行不必要的 NULL或者null 检查\"的例子\n<例子1>\na = \"dog\";\nif (a != null){\n\treturn a;\n}这段代码中的对象a已经是确定的值\"dog\"，所以if条件句的判断\"a != null\"是不必要的，所以这个被判定为\"避免对象进行不必要的 NULL或者null 检查\"\n\n\n<例子2>\nif (authenticatedUserId != null && !authenticatedUserId.isEmpty() && userGroupManager!=null){\n\treturn authenticatedUserId;\n}这段代码中的\"authenticatedUserId != null\"和\"!authenticatedUserId.isEmpty()\"都是对\"authenticatedUserId\"的空判断，重复了，所以这个被判定为\"避免对象进行不必要的 NULL或者null 检查\"\n\n\n<例子3>\nList list = new ArrayList<>();\nif (list != null) {\n list.add(1);\n}这段代码中的list已经被初始化，不需要进行 null 检查，所以这个被判定为\"避免对象进行不必要的 NULL或者null 检查\"\n\n\n<例子4>\nif (this.type != null && this.type.getName() != null) {\n\tSystem.out.println(\"Type name is not null\");\n}这段代码中的对象type已经检查过非null，再次检查getName()是否为null是不必要的，所以这个被判定为\"避免对象进行不必要的 NULL或者null 检查\"\n\n\n\n<例子5>\nif (\"dog\".equals(null)){\n\treturn a;\n}这段代码中的\"dog\"是个确定的字符串，不需要进行null 检查，所以这个被判定为\"避免对象进行不必要的 NULL或者null 检查\"\n\n\n<例子6>\nInteger num = 10;\nif (num != null) {\n System.out.println(num);\n}这段代码中的num 已经被初始化，不需要进行 null 检查，所以这个被判定为\"避免对象进行不必要的 NULL或者null 检查\"\n", + "no_example": "### 不能被判定为\"避免对象进行不必要的 NULL或者null 检查\"的例子\n<例子1>\nCat cat = catService.get(1);\nif (cat != null){\n\tretrun cat;\n}这段代码中的对象\"cat\"是通过service获取到的，不确定是否为空，所以if条件句的判断的\"cat != null\"是必要的，所以这个不能被判定为\"避免对象进行不必要的 NULL或者null 检查\"\n" + }, + { + "id": 17, + "text": "避免 finally 块中出现 return", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：避免 finally 块中出现 return；修复方案：无需修复", + "yes_example": "### 被判定为\"避免 finally 块中出现 return\"的例子\n<例子1>\npublic void getPersion() {\n\ttry {\n\t\tPersion persion = persionService.getPersion(1);\n\t\tif (persion != null){ \n\t\t\treturn persion;\n\t\t}\n\t} finally {\n\t\treturn null;\n\t}\n}这段代码中的finally语句块内容包含\"return\"，所以这个被判定为\"避免 finally 块中出现 return\"\n", + "no_example": "### 不能被判定为\"避免 finally 块中出现 return\"的例子\n<例子1>\npublic void getPersion() {\n\ttry {\n\t\tPersion persion = persionService.getPersion(1);\n\t\tif (persion != null){ \n\t\t\treturn persion;\n\t\t}\n\t} finally {\n\t\tLOGGER.info(PERSION_NOT_EXIT);\n\t}\n}这段代码中的finally语句块中内容不包含\"return\"，所以这个不能被判定为\"避免 finally 块中出现 return\"\n" + }, + { + "id": 18, + "text": "避免空的 static 初始化", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：避免空的 static 初始化；对应Fixer：EmptyInitializerFixer；修复方案：删除整个空初始化块", + "yes_example": "### 被判定为\"避免空的 static 初始化\"的例子\n<例子1>\npublic class PetValidator implements Validator {\n\tstatic {\n\n\t}\n}这段代码中的static语句块没有内容，是空的，所以这个被判定为\"避免空的 static 初始化\"\n\n\n<例子2>\npublic class Persion {\n\tstatic {\n\t\t// 初始化的静态块\n\t}\n}这段代码中的static语句块是有内容的，不是空的，但是static初始化语句块中只有注释代码，没有实际的逻辑，所以这个被判定为\"避免空的 static 初始化\"\n", + "no_example": "### 不能被判定为\"避免空的 static 初始化\"的例子\n<例子1>\npublic class Cat {\n\tstatic {\n\t\t// 初始化的静态块\n\t\tcat = null;\n\t}\n}这段代码中的static语句块是有内容的，不是空的，且static初始化语句块中有非注释代码，有实际的逻辑，所以这个不能被判定为\"避免空的 static 初始化\"\n" + }, + { + "id": 19, + "text": "避免日历类用法不当风险", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：避免日历类用法不当风险；修复方案：使用Java 8 及以上版本中的 java.time 包的LocalDate", + "yes_example": "### 被判定为\"避免日历类用法不当风险\"的例子\n<例子1>\nprivate static final Calendar calendar = new GregorianCalendar(2020, Calendar.JANUARY, 1);\n这段代码中的Calendar和GregorianCalendar是线程不安全的，所以这个被判定为\"避免日历类用法不当风险\"\n", + "no_example": "### 不能被判定为\"避免日历类用法不当风险\"的例子\n<例子1>\nprivate static final LocalDate calendar = LocalDate.of(2020, 1, 1);\n这段代码中的LocalDate使用的是Java 8 及以上版本中的 java.time 包，LocalDate 是不可变的并且是线程安全的，不会有线程安全和性能方面的问题，所以这个不能被判定为\"避免日历类用法不当风险\"\n" + }, + { + "id": 21, + "text": "使用集合转数组的方法，必须使用集合的toArray(T[]array)，传入的是类型完全一样的数组，大小就是list.size()", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：使用集合转数组的方法，必须使用集合的toArray(T[]array)，传入的是类型完全一样的数组，大小就是list.size()；对应Fixer：ClassCastExpWithToArrayF ixer；修复方案：使用集合的toArray(T[]array)，且传入的是类型完全一样的数组", + "yes_example": "### 被判定为\"使用集合转数组的方法，必须使用集合的toArray(T[]array)，传入的是类型完全一样的数组，大小就是list.size()\"的例子\n<例子1>\nList stringList = new ArrayList<>();\nstringList.add(\"Apple\");\nstringList.add(\"Banana\");\nObject[] objectArray = stringList.toArray(new Object[5]);\n这段代码使用集合转数组的方法的时候使用了toArray(new Object[5])，但是传入的数组类型不一致，所以这个被判定为\"使用集合转数组的方法，必须使用集合的toArray(T[]array)，传入的是类型完全一样的数组，大小就是list.size()\"\n", + "no_example": "### 不能被判定为\"使用集合转数组的方法，必须使用集合的toArray(T[]array)，传入的是类型完全一样的数组，大小就是list.size()\"的例子\n<例子1>\nList stringList = new ArrayList<>();\nstringList.add(\"Apple\");\nstringList.add(\"Banana\");\nString[] stringArray = stringList.toArray(new String[stringList.size()]);\n这段代码使用集合转数组的方法的时候使用了toArray(new String[stringList.size()])，传入的是类型完全一样的数组，所以这个不能被判定为\"使用集合转数组的方法，必须使用集合的toArray(T[]array)，传入的是类型完全一样的数组，大小就是list.size()\"\n" + }, + { + "id": 22, + "text": "禁止在 equals()中使用 NULL或者null 做比较", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：禁止在 equals()中使用 NULL或者null 做比较；对应Fixer：EqualsNullFixer；修复方案：使用Object的判空函数 做比较", + "yes_example": "### 被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"的例子\n<例子1>\nif (\"test\".equals(null)) {\n\tSystem.out.println(\"test\");\n}这段代码中if条件中的代码\"test\".equals(null)使用equals()函数与null进行了比较，所以这个被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"\n\n\n<例子2>\nif (!rangeValues[1].equals(\"null\")) {\n\tmaxValue = new BigDecimal(rangeValues[1]);\n}这段代码中if条件中的代码!rangeValues[1].equals(\"null\")使用equals()函数与Nnull进行了比较，所以这个被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"\n\n\n<例子3>\nString str1 = \"example\";\nif (str1.equals(\"null\")) {\n System.out.println(\"str1 is null\");\n}这段代码中if条件中的代码str1.equals(null)使用equals()函数与null进行了比较，所以这个被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"\n\n\n<例子4>\nString str3 = \"example\";\nif (str3 != null && str3.equals(\"null\")) {\n System.out.println(\"str3 is null\");\n}这段代码中if条件中的代码str3.equals(\"null\")使用equals()函数与\"null\"进行了比较，所以这个被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"\n\n\n<例子5>\nInteger num1 = 10;\nif (num1.equals(null)) {\n System.out.println(\"num1 is null\");\n}这段代码中if条件中的代码num1.equals(null)使用equals()函数与\"null\"进行了比较，所以这个被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"\n\n\n<例子6>\nObject obj = new Object();\nif (obj.equals(null)) {\n System.out.println(\"obj is null\");\n}这段代码中if条件中的代码obj.equals(null)使用equals()函数与\"null\"进行了比较，所以这个被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"\n", + "no_example": "### 不能被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"的例子\n<例子1>\na = \"test\";\nif (a.equals(\"test\")) {\n\tSystem.out.println(\"test\");\n}这段代码中if条件中的代码a.equals(\"test\")使用equals()函数与\"test\"进行了比较，所以这个不能被判定为\"禁止在 equals()中使用 NULL或者null 做比较\"\n" + }, + { + "id": 23, + "text": "switch 语句块不能为空", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：switch 语句块不能为空；对应Fixer：EmptySwitchStatementsFix；修复方案：删除整个空 switch 语句块", + "yes_example": "### 被判定为\"switch 语句块不能为空\"的例子\n<例子1>\nswitch (number) {\n\t\n}这段代码是一个switch语句块，但是里面没有内容，所以这个被判定为\"switch 语句块不能为空\"\n\n\n<例子2>\nswitch (number) {\n\t// 这是一个switch语句块\n}这段代码是一个switch语句块，里面虽然有内容，但是内容仅仅是注释内容，没有实际的逻辑，所以这个被判定为\"switch 语句块不能为空\"\n", + "no_example": "### 不能被判定为\"switch 语句块不能为空\"的例子\n<例子1>\nswitch (number) {\n\tcase 1:\n\t\tSystem.out.println(\"Number one\");\n\t\tbreak;\n\tdefault:\n\t\tSystem.out.println(\"This is the default block, which is incorrectly placed here.\");\n\t\tbreak;\n}这段代码是一个switch语句块，里面有内容，而且内容里有非注释的代码，有实际的逻辑，所以这个不能被判定为\"switch 语句块不能为空\"\n" + }, + { + "id": 24, + "text": "变量/方法/类 命名，要有效清晰表示对应的实际含义", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：变量/方法/类 命名，要有效清晰表示对应的实际含义；修复方案：修改成合适的变量/方法/类名，有效表示对应的实体含义", + "yes_example": "### 被判定为\"变量/方法/类 命名，要有效清晰表示对应的实际含义\"的例子\n<例子1>\nint a = 1;\nif (a == 1) {\n\treturn a;\n}这段代码中的变量a的命名无法从命名中理解a代表什么，所以这个被判定为\"变量/方法/类 命名，要有效清晰表示对应的实际含义\"\n\n\n<例子2>\nStringBuilder sb = new StringBuilder();\nsb.append(\"{!\").append(tag).append(\"=\").append(tagField);\n这段代码中的变量sb的命名无法从命名中理解a代表什么，所以这个被判定为\"变量/方法/类 命名，要有效清晰表示对应的实际含义\"\n\n\n<例子3>\nPage findAll(Pageable pageable);\n这段代码findAll是一个interface声明，接口名findAll无法清晰的理解这个接口要做什么，所以这个被判定为\"变量/方法/类 命名，要有效清晰表示对应的实际含义\"\n", + "no_example": "### 不能被判定为\"变量/方法/类 命名，要有效清晰表示对应的实际含义\"的例子\n<例子1>\nint persionNum = 5;\nif (persionNum > 3) {\n\treturn True;\n}这段代码中的变量persionNum的命名可以比较清楚知道persionNum代表人员的数量，所以这个不能被判定为\"变量/方法/类 命名，要有效清晰表示对应的实际含义\"\n" + }, + { + "id": 25, + "text": "判断逻辑应简洁，不能有重复或者无意义的判断", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：判断逻辑应简洁，不能有重复或者无意义的判断；修复方案：简化或删除一些重复的，无意义的判断逻辑", + "yes_example": "### 被判定为\"判断逻辑应简洁，不能有重复或者无意义的判断\"\n<例子1>\nif (task == null) {\n\ttask = 1\n\tif (task != null) {\n\t\tInteger ok = 0;\n\t\tif (task == 1) {\n\t\t\tok=1;\n\t\t}\n\t\tif (ok == 0) {\n\t\t\ttask = null;\n\t\t}\n\t}\n}这段代码中的关于变量ok的判断逻辑不够简洁，变量ok其实可以被去掉进行逻辑简化，可以被简化为\nif (task == null) {\n\ttask = 1\n\tif (task != null) {\n\t\tif (task == 1) {\n\t\t task = null;\n\t\t}\n\t}\n}，所以这个被判定为\"判断逻辑应简洁，不能有重复或者无意义的判断\"\n\n\n<例子2>\nif (!(a == 0) && a == 1 || a == 2){\n\treturn True;\n}这段代码的if判断逻辑过于复杂，可简化为if (0\n\n<例子3>\na = 1;\nif (a > 1){\n\treturn True;\n}这段代码的变量a是确定的值1，这样就不会出现a > 1，所以if的判断无意义，所以这个被判定为\"判断逻辑应简洁，不能有重复或者无意义的判断\"\n", + "no_example": "### 不能被判定为\"判断逻辑应简洁，不能有重复或者无意义的判断\"\n<例子1>\na = 1;\nb = 2;\nif (0 < a ≤ 2){\n\treturn a + b;\n}这段代码的判断条件比较简洁明了，无重复的判断，所以这个不能被判定为\"判断逻辑应简洁，不能有重复或者无意义的判断\"\n" + }, + { + "id": 26, + "text": "代码结构冗余，避免重复代码", + "language": "java", + "file_path": "", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：代码结构冗余，避免重复代码；修复方案：忽略", + "yes_example": "### 被判定为\"代码结构冗余，避免重复代码\"的例子\n<例子1>\npublic void trigger(String executionId, Map processVariables) {\n commandExecutor.execute(new TriggerCmd(executionId, processVariables));\n}\npublic void trigger(String executionId, Map processVariables, Map transientVariables) {\n\tcommandExecutor.execute(new TriggerCmd(executionId, processVariables, transientVariables)); \n}这段代码中的2个trigger存在代码都是触发器的逻辑，存在重复，可以合并成1个函数，所以这个被判定为\"代码结构冗余，避免重复代码\"\n\n\n<例子2>\npublic int sum(int a, int b){\n\treturn a + b;\n}\npublic int merge(int a, int b){\n\treturn a + b;\n}这段代码中的2个函数sum和merge的逻辑是一样的，可以合并成1个函数，所以这个被判定为\"代码结构冗余，避免重复代码\"\n", + "no_example": "### 不能被判定为\"代码结构冗余，避免重复代码\"的例子\n<例子1>\npublic int append(int a, int b){\n\treturn a + b;\n}\npublic int add(int a, int b){\n\treturn (a + b) - b;\n}这段代码中的2个函数sum和add的代码结构看上去差不多，但是具体的处理逻辑是不一样的，所以这个不能被判定为\"代码结构冗余，避免重复代码\"\n" + }, + { + "id": 27, + "text": "不能有多余的分号", + "file_path": "\"\"", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：多余的分号；修复方案：删除多余的分号", + "yes_example": "### 被判定为\"不能有多余的分号\"的例子\n<例子1>\npublic void trigger(String executionId, Map processVariables) {\n commandExecutor.execute(new TriggerCmd(executionId, processVariables));\n}\n;\na = 1;\nb = 2;\nsum = a + b;\n这段代码中包含一个多余的分号\";\"，所以这个被判定为\"不能有多余的分号\"\n", + "no_example": "### 不能被判定为\"不能有多余的分号\"的例子\n<例子1>\nwhile (True) {\n\ta = a + 1;\n\tbreak;\n}这段代码每个分号都是必须要的，所以这个能被判定为\"不能有多余的分号\"\n" + }, + { + "id": 28, + "text": "非线程安全的 SimpleDateFormat 使用，必须在函数或代码块级别使用synchronized", + "file_path": "\"\"", + "start_line": 0, + "end_line": 0, + "detail": "缺陷类型：非线程安全的 SimpleDateFormat 使用；修复方案：在函数或代码块级别加上synchronized修饰 或 使用其他线程安全的方式", + "yes_example": "### 被判定为\"非线程安全的 SimpleDateFormat 使用，必须在函数或代码块级别使用synchronized\"的例子\n<例子1>\npublic void formatDate(Date date) {\n\tSimpleDateFormat sdf = new SimpleDateFormat(\"yyyy-MM-dd\");\n\tSystem.out.println(\"Formatted date: \" + sdf.format(date));\n}这段代码中的函数formatDate在未使用synchronized同步修饰的情况下使用了SimpleDateFormat，这是线程不安全的，所以这个被判定为\"非线程安全的 SimpleDateFormat 使用，必须在函数或代码块级别使用synchronized\"\n", + "no_example": "### 不能被判定为\"非线程安全的 SimpleDateFormat 使用，必须在函数或代码块级别使用synchronized\"的例子\n<例子1>\npublic synchronized void formatDate(Date date) {\n\tSimpleDateFormat sdf = new SimpleDateFormat(\"yyyy-MM-dd\");\n\tSystem.out.println(\"Formatted date: \" + sdf.format(date));\n}这段代码是在synchronized同步块对函数'formatDate'进行保护，保证了线程安全，所以这个不能被判定为\"非线程安全的 SimpleDateFormat 使用，必须在函数或代码块级别使用synchronized\"\n" + } +] \ No newline at end of file diff --git a/metagpt/ext/cr/utils/__init__.py b/metagpt/ext/cr/utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/metagpt/ext/cr/utils/cleaner.py b/metagpt/ext/cr/utils/cleaner.py new file mode 100644 index 000000000..b16ac8119 --- /dev/null +++ b/metagpt/ext/cr/utils/cleaner.py @@ -0,0 +1,68 @@ +"""Cleaner.""" + +from unidiff import Hunk, PatchedFile, PatchSet + +from metagpt.logs import logger + + +def rm_patch_useless_part(patch: PatchSet, used_suffix: list[str] = ["java"]) -> PatchSet: + new_patch = PatchSet("") + useless_files = [] + for pfile in patch: + suffix = str(pfile.target_file).split(".")[-1] + if suffix not in used_suffix or pfile.is_removed_file or "test" in pfile.target_file.casefold(): + useless_files.append(pfile.path) + continue + new_patch.append(pfile) + logger.info(f"total file num: {len(patch)}, used file num: {len(new_patch)}, useless_files: {useless_files}") + return new_patch + + +def add_line_num_on_patch(patch: PatchSet, start_line_num: int = 1) -> PatchSet: + new_patch = PatchSet("") + lineno = start_line_num + for pfile in patch: + new_pfile = PatchedFile( + source=pfile.source_file, + target=pfile.target_file, + source_timestamp=pfile.source_timestamp, + target_timestamp=pfile.target_timestamp, + ) + for hunk in pfile: + arr = [str(line) for line in hunk] + new_hunk = Hunk( + src_start=hunk.source_start, + src_len=hunk.source_length, + tgt_start=hunk.target_start, + tgt_len=hunk.target_length, + section_header=hunk.section_header, + ) + + for line in arr: + # if len(line) > 0 and line[0] in ["+", "-"]: + # line = f"{lineno} {line}" + # lineno += 1 + line = f"{lineno} {line}" + lineno += 1 + new_hunk.append(line) + new_pfile.append(new_hunk) + new_patch.append(new_pfile) + return new_patch + + +def get_code_block_from_patch(patch: PatchSet, code_start_line: str, code_end_line: str) -> str: + line_arr = str(patch).split("\n") + code_arr = [] + add_line_tag = False + for line in line_arr: + if line.startswith(f"{code_start_line} "): + add_line_tag = True + + if add_line_tag: + new_line = " ".join(line.split(" ")[1:]) # rm line-no tag + code_arr.append(new_line) + + if line.startswith(f"{code_end_line} "): + add_line_tag = False + + return "\n".join(code_arr) diff --git a/metagpt/ext/cr/utils/schema.py b/metagpt/ext/cr/utils/schema.py new file mode 100644 index 000000000..e7b08792f --- /dev/null +++ b/metagpt/ext/cr/utils/schema.py @@ -0,0 +1,20 @@ +from typing import Literal + +from pydantic import BaseModel, Field + + +class Point(BaseModel): + id: int = Field(default=0, description="ID of the point.") + text: str = Field(default="", description="Content of the point.") + language: Literal["python", "java"] = Field( + default="python", description="The programming language that the point corresponds to." + ) + file_path: str = Field(default="", description="The file that the points come from.") + start_line: int = Field(default=0, description="The starting line number that the point refers to.") + end_line: int = Field(default=0, description="The ending line number that the point refers to.") + detail: str = Field(default="", description="File content from start_line to end_line.") + yes_example: str = Field(default="", description="yes of point examples") + no_example: str = Field(default="", description="no of point examples") + + def rag_key(self) -> str: + return self.text diff --git a/metagpt/tools/libs/cr.py b/metagpt/tools/libs/cr.py new file mode 100644 index 000000000..7f9a4716c --- /dev/null +++ b/metagpt/tools/libs/cr.py @@ -0,0 +1,90 @@ +import json +from pathlib import Path +from typing import Optional + +import aiofiles +from unidiff import PatchSet + +import metagpt.ext.cr +from metagpt.ext.cr.actions.code_review import CodeReview as CodeReview_ +from metagpt.ext.cr.actions.modify_code import ModifyCode +from metagpt.ext.cr.utils.schema import Point +from metagpt.tools.libs.browser import Browser +from metagpt.tools.tool_registry import register_tool +from metagpt.utils.report import EditorReporter + + +@register_tool(tags=["codereview"], include_functions=["review", "fix"]) +class CodeReview: + """Review and fix the patch content from the pull request URL or a file.""" + + async def review( + self, + patch_path: str, + cr_output_file: str, + cr_point_file: Optional[str] = None, + ) -> str: + """Review a PR and save code review comments. + + Args: + patch_path: The local path of the patch file or the url of the pull request. Example: "/data/xxx-pr-1.patch", "https://github.com/xx/XX/pull/1362" + cr_output_file: Output file path where code review comments will be saved. Example: "cr/xxx-pr-1.json" + cr_point_file: File path for specifying code review points. Defaults to a predefined file. + """ + patch = await self._get_patch_content(patch_path) + cr_point_file = cr_point_file if cr_point_file else Path(metagpt.ext.cr.__file__).parent / "points.json" + async with aiofiles.open(cr_point_file, "rb") as f: + cr_point_content = await f.read() + cr_points = [Point(**i) for i in json.loads(cr_point_content)] + + async with EditorReporter(enable_llm_stream=True) as reporter: + src_path = cr_output_file + cr_output_path = Path(cr_output_file) + await reporter.async_report( + {"type": "CodeReview", "src_path": src_path, "filename": cr_output_path.name}, "meta" + ) + comments = await CodeReview_().run(patch, cr_points) + cr_output_path.parent.mkdir(exist_ok=True, parents=True) + async with aiofiles.open(cr_output_path, "w") as f: + await f.write(json.dumps(comments, ensure_ascii=False)) + await reporter.async_report(cr_output_path) + + return f"The number of defects: {len(comments)} and the comments are stored in {cr_output_file}" + + async def fix( + self, + patch_path: str, + cr_file: str, + output_dir: str, + ) -> str: + """Fix the patch content based on code review comments. + + Args: + patch_path: The local path of the patch file or the url of the pull request. + cr_file: File path where code review comments are stored. + output_dir: File path where code review comments are stored. + """ + patch = await self._get_patch_content(patch_path) + async with aiofiles.open(cr_file, "r") as f: + comments = json.loads(await f.read()) + await ModifyCode(pr="").run(patch, comments, output_dir) + return f"The fixed patch files store in {output_dir}" + + async def _get_patch_content(self, patch_path): + if patch_path.startswith(("https://", "http://")): + # async with aiohttp.ClientSession(trust_env=True) as client: + # async with client.get(f"{patch_path}.diff", ) as resp: + # patch_file_content = await resp.text() + browser = Browser() + browser.proxy = {"server": "http://127.0.0.1:20172"} + async with browser: + await browser.goto(f"{patch_path}.diff") + patch_file_content = await browser.page.content() + + else: + async with aiofiles.open(patch_path) as f: + patch_file_content = await f.read() + await EditorReporter().async_report(patch_path) + + patch: PatchSet = PatchSet(patch_file_content) + return patch