actions-rust-lang / setup-rust-toolchain

Setup a specific Rust toolchain with extra features like problem matchers
MIT License
193 stars 33 forks source link

Problem matcher is slightly off #17

Closed max-sixty closed 1 year ago

max-sixty commented 1 year ago

This seems like the "fault" of cargo fmt rather than this action, but FYI — the lines for the problem matcher are slightly off:

https://github.com/PRQL/prql/pull/2558/files#diff-d8cf470395314450935c363c0f82ef23888873d109e7df81e131728c66265e84L128-L132

image

It looks like this is because the line number reported by rustfmt is from the top of the code it shows. I think technically the selection for GH should match to the lines marked with -, rather than the 128:

Diff in /Users/maximilian/workspace/prql/prql-compiler/src/lib.rs at line 128:
     parser::parse(prql)
         .and_then(semantic::resolve)
         .and_then(|rq| sql::compile(rq, options))
-        .map_err(
-            error::downcast
-        )
+        .map_err(error::downcast)
         .map_err(|e| e.composed("", prql, options.color))
 }
max-sixty commented 1 year ago

Just looked at how these work — looks like it's not sophisticated enough from GH's side to handle this well: https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md

So feel free to close...

jonasbb commented 1 year ago

Problem matcher are not powerful enough to alter the extracted data in any way. They are only as powerful as regex and have to work with the line information as printed by rustfmt. Any more complex processing would require it's own action.

You can remove the problem matcher if the inaccuracy is a problem.

echo "::remove-matcher owner=rustfmt::"

There is an action to show the rustfmt results as an execution summary document. It still uses the line numbers as printed by rustfmt. https://github.com/actions-rust-lang/rustfmt/