Skip to content

Support mozlint diffs #437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
La0 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support mozlint diffs #437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 39 additions & 5 deletions bot/code_review_bot/tasks/lint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,20 @@ def __init__(
message,
check,
revision,
**kwargs
diff=None,
):
base_line = lineno and int(lineno) or 0
nb_lines = 1

if diff:
try:
base_line, nb_lines = self.parse_diff(diff, base_line)

# Add diff to message
message += f"\n```\n{diff}\n```"
except Exception as e:
logger.warning(f"Failed to parse diff: {e}")

# Use analyzer name when check is not provided
# This happens for analyzers who only have one rule
if check is None:
Expand All @@ -44,17 +56,38 @@ def __init__(
analyzer,
revision,
path,
line=(
lineno and int(lineno) or 0
), # mozlint sometimes produce strings here
nb_lines=1,
line=base_line,
nb_lines=nb_lines,
check=check,
column=column,
level=Level(level),
message=message,
)
self.linter = linter

def parse_diff(self, diff, base_line):
"""
Parse an optional Mozlint diff to get the lines from the original patch
"""
changes = list(
filter(
None,
[
(i, c[0]) if c and c[0] in ("-", "+") else None
for i, c in enumerate(diff.splitlines(), start=base_line)
],
)
)
assert changes, "No changes in diff"

# First change has the position of the issue's top line
base_line, _ = changes[0]

# We only consider the old lines as being in patch
nb_lines = len([c for c in changes if c[1] == "-"]) or 1

return base_line, nb_lines

def is_disabled_check(self):
"""
Some checks are disabled:
Expand Down Expand Up @@ -126,6 +159,7 @@ def parse_issues(self, artifacts, revision):
linter=issue["linter"],
message=issue["message"],
check=issue["rule"],
diff=issue.get("diff"),
)
for artifact in artifacts.values()
for _, path_issues in artifact.items()
Expand Down
272 changes: 272 additions & 0 deletions bot/tests/fixtures/mozlint_rustfmt_issues.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,272 @@
{
"/build/checkout/": [
{
"linter": "rust",
"path": "/build/checkout/",
"message": "Reformat rust",
"lineno": 0,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "Warning: can't set `binop_separator = Back`, unstable features are only available in nightly channel.\nWarning: can't set `match_block_trailing_comma = true`, unstable features are only available in nightly channel.",
"relpath": "."
}
],
"/build/checkout/tools/fuzzing/rust/src/lib.rs": [
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 16,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " use tempfile::Builder;\n\n fn eat_lmdb_err<T>(value: Result<T, rkv::StoreError>) -> Result<Option<T>, rkv::StoreError> {\n- // Should trigger rustfmt in patch\n- match value {\n+ // Should trigger rustfmt in patch\n+ match value {\n Ok(value) => Ok(Some(value)),\n Err(rkv::StoreError::LmdbError(_)) => Ok(None),\n Err(err) => {",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 24,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " println!(\"Not a crash, but an error outside LMDB: {}\", err);\n println!(\"A refined fuzzing test, or changes to RKV, might be required.\");\n Err(err)\n- },\n+ }\n }\n }",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 44,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "\n let &mut builder = rkv::Rkv::environment_builder().set_max_dbs(2);\n let env = rkv::Rkv::from_env(Path::new(\".\"), builder).unwrap();\n- let store = env.open_single(\"test\", rkv::StoreOptions::create()).unwrap();\n+ let store = env\n+ .open_single(\"test\", rkv::StoreOptions::create())\n+ .unwrap();\n\n let reader = env.read().unwrap();\n eat_lmdb_err(store.get(&reader, &[0])).unwrap();",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 78,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " pub extern \"C\" fn fuzz_rkv_key_write(raw_data: *const u8, size: libc::size_t) -> libc::c_int {\n let data = unsafe { std::slice::from_raw_parts(raw_data as *const u8, size as usize) };\n\n- let root = Builder::new().prefix(\"fuzz_rkv_key_write\").tempdir().unwrap();\n+ let root = Builder::new()\n+ .prefix(\"fuzz_rkv_key_write\")\n+ .tempdir()\n+ .unwrap();\n fs::create_dir_all(root.path()).unwrap();\n\n let env = rkv::Rkv::new(root.path()).unwrap();",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 85,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "- let store = env.open_single(\"test\", rkv::StoreOptions::create()).unwrap();\n+ let store = env\n+ .open_single(\"test\", rkv::StoreOptions::create())\n+ .unwrap();\n\n let mut writer = env.write().unwrap();\n // Some data are invalid values, and are handled as store errors.",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 96,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " pub extern \"C\" fn fuzz_rkv_val_write(raw_data: *const u8, size: libc::size_t) -> libc::c_int {\n let data = unsafe { std::slice::from_raw_parts(raw_data as *const u8, size as usize) };\n\n- let root = Builder::new().prefix(\"fuzz_rkv_val_write\").tempdir().unwrap();\n+ let root = Builder::new()\n+ .prefix(\"fuzz_rkv_val_write\")\n+ .tempdir()\n+ .unwrap();\n fs::create_dir_all(root.path()).unwrap();\n\n let env = rkv::Rkv::new(root.path()).unwrap();",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 103,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "- let store = env.open_single(\"test\", rkv::StoreOptions::create()).unwrap();\n+ let store = env\n+ .open_single(\"test\", rkv::StoreOptions::create())\n+ .unwrap();\n\n let mut writer = env.write().unwrap();\n let string = String::from_utf8_lossy(data);",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 129,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "\n #[no_mangle]\n pub extern \"C\" fn fuzz_rkv_calls(raw_data: *const u8, size: libc::size_t) -> libc::c_int {\n- let mut fuzz = unsafe { std::slice::from_raw_parts(raw_data as *const u8, size as usize).to_vec() };\n+ let mut fuzz =\n+ unsafe { std::slice::from_raw_parts(raw_data as *const u8, size as usize).to_vec() };\n\n fn maybe_do(fuzz: &mut Vec<u8>, f: impl FnOnce(&mut Vec<u8>) -> ()) -> Option<()> {\n match fuzz.pop().map(|byte| byte % 2) {",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 136,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " Some(0) => Some(f(fuzz)),\n- _ => None\n+ _ => None,\n }\n }",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 141,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "- fn maybe_abort(fuzz: &mut Vec<u8>, read: rkv::Reader) -> Result<(), rkv::StoreError> {\n+ fn maybe_abort(fuzz: &mut Vec<u8>, read: rkv::Reader) -> Result<(), rkv::StoreError> {\n match fuzz.pop().map(|byte| byte % 2) {\n Some(0) => Ok(read.abort()),\n- _ => Ok(())\n+ _ => Ok(()),\n }\n };",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 148,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "- fn maybe_commit(fuzz: &mut Vec<u8>, write: rkv::Writer) -> Result<(), rkv::StoreError> {\n+ fn maybe_commit(fuzz: &mut Vec<u8>, write: rkv::Writer) -> Result<(), rkv::StoreError> {\n match fuzz.pop().map(|byte| byte % 3) {\n Some(0) => write.commit(),\n Some(1) => Ok(write.abort()),",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 152,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "- _ => Ok(())\n+ _ => Ok(()),\n }\n };",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 187,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " });\n\n let env = rkv::Rkv::from_env(root.path(), builder).unwrap();\n- let store = env.open_single(\"test\", rkv::StoreOptions::create()).unwrap();\n+ let store = env\n+ .open_single(\"test\", rkv::StoreOptions::create())\n+ .unwrap();\n\n loop {\n match fuzz.pop().map(|byte| byte % 4) {",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 194,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " Some(0) => {\n let key = match get_key(&mut fuzz) {\n Some(value) => value,\n- None => break\n+ None => break,\n };\n let value = match get_value(&mut fuzz) {\n Some(value) => value,",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 201,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": "- None => break\n+ None => break,\n };\n let mut writer = env.write().unwrap();\n eat_lmdb_err(store.put(&mut writer, key, &rkv::Value::Str(value))).unwrap();",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 207,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " Some(1) => {\n let key = match get_key(&mut fuzz) {\n Some(value) => value,\n- None => break\n+ None => break,\n };\n let mut reader = env.read().unwrap();\n eat_lmdb_err(store.get(&mut reader, key)).unwrap();",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 216,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " Some(2) => {\n let key = match get_key(&mut fuzz) {\n Some(value) => value,\n- None => break\n+ None => break,\n };\n let mut writer = env.write().unwrap();\n eat_lmdb_err(store.delete(&mut writer, key)).unwrap();",
"relpath": "tools/fuzzing/rust/src/lib.rs"
},
{
"linter": "rust",
"path": "/build/checkout/tools/fuzzing/rust/src/lib.rs",
"message": "Reformat rust",
"lineno": 229,
"column": null,
"hint": null,
"source": null,
"level": "warning",
"rule": null,
"lineoffset": null,
"diff": " let n = fuzz.pop().unwrap_or(1) as usize;\n builder.set_map_size(1_048_576 * (n % 100)); // 1,048,576 bytes, i.e. 1MiB.\n }\n- _ => {\n- break\n- }\n+ _ => break,\n }\n }\n\nWarning: can't set `binop_separator = Back`, unstable features are only available in nightly channel.\nWarning: can't set `match_block_trailing_comma = true`, unstable features are only available in nightly channel.\nWarning: can't set `binop_separator = Back`, unstable features are only available in nightly channel.\nWarning: can't set `match_block_trailing_comma = true`, unstable features are only available in nightly channel.\n",
"relpath": "tools/fuzzing/rust/src/lib.rs"
}
]
}
Loading