Skip to content

Commit

Permalink
bot: Support mozlint diffs, fixes mozilla#31
Browse files Browse the repository at this point in the history
  • Loading branch information
Bastien Abadie committed Feb 18, 2020
1 parent 737af4e commit 0845149
Show file tree
Hide file tree
Showing 3 changed files with 381 additions and 5 deletions.
44 changes: 39 additions & 5 deletions bot/code_review_bot/tasks/lint.py
View file
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
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

0 comments on commit 0845149

Please sign in to comment.