JOJ/JOJ3
24
5
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

clang-tidy parser and executor #26

Merged
张泊明518370910136 merged 26 commits from clang-tidy into master 2024-05-18 02:50:13 +08:00
Conversation 25 Commits 26 Files Changed 9 +9 -7
3 changed files with 9 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 07ef611f95 - Show all commits

2
examples/clang-tidy/sillycode

@ -1 +1 @@
Subproject commit 422868c1c6427ce73e665477b40922c72ccc14e5 Subproject commit 580d9c9e20ae887551af2f87483d82cf6efa6f6d

8
internal/parsers/clang_tidy/convert.go
View File

@ -109,14 +109,14 @@ func group_messages(messages []ClangMessage) []ClangMessage {
return grouped_messages return grouped_messages
} }
func convert_paths_to_relative(messages *[]ClangMessage) { func convert_paths_to_relative(messages *[]ClangMessage, conf Conf) {
currentDir := "/w" currentDir := conf.RootDir
zjc_he marked this conversation as resolved Outdated
张泊明518370910136 commented 2024-05-07 02:46:16 +08:00
Outdated
Review
Copy Link

I think it should be part of the config file with default value "/w". Since the work dir in the sandbox can also be changed, it should not be hardcoded.

I think it should be part of the config file with default value `"/w"`. Since the work dir in the sandbox can also be changed, it should not be hardcoded.
for i := range *messages { for i := range *messages {
(*messages)[i].filepath, _ = filepath.Rel(currentDir, (*messages)[i].filepath) (*messages)[i].filepath, _ = filepath.Rel(currentDir, (*messages)[i].filepath)
} }
} }
func parse_lines(lines []string) []ClangMessage { func parse_lines(lines []string, conf Conf) []ClangMessage {
messages := make([]ClangMessage, 0) messages := make([]ClangMessage, 0)
for _, line := range lines { for _, line := range lines {
if is_ignored(string(line)) { if is_ignored(string(line)) {
@ -129,6 +129,6 @@ func parse_lines(lines []string) []ClangMessage {
messages = append(messages, message) messages = append(messages, message)
} }
} }
convert_paths_to_relative(&messages) convert_paths_to_relative(&messages, conf)
return group_messages(messages) return group_messages(messages)
} }

6
internal/parsers/clang_tidy/parser.go
View File

@ -14,7 +14,8 @@ type Match struct {
} }
type Conf struct { type Conf struct {
zjc_he marked this conversation as resolved Outdated
张泊明518370910136 commented 2024-05-07 14:59:19 +08:00
Outdated
Review
Copy Link

Only a little confused about it. Does it mean /w by default as we run in linux? Or whether clang-tidy has some output format.

Only a little confused about it. Does it mean `/w` by default as we run in linux? Or whether clang-tidy has some output format.
张佳澈520370910044 commented 2024-05-07 15:35:07 +08:00
Outdated
Review
Copy Link

Sorry that was a mistake

Sorry that was a mistake
Score int Score int `default:"100"`
RootDir string `default:"\\w"`
Matches []Match Matches []Match
} }
@ -24,7 +25,7 @@ func Parse(executorResult stage.ExecutorResult, conf Conf) stage.ParserResult {
stdout := executorResult.Files["stdout"] stdout := executorResult.Files["stdout"]
lines := strings.SplitAfter(stdout, "\n") lines := strings.SplitAfter(stdout, "\n")
messages := parse_lines(lines) messages := parse_lines(lines, conf)
formatted_messages := format(messages) formatted_messages := format(messages)
zjc_he marked this conversation as resolved Outdated
张泊明518370910136 commented 2024-05-07 15:02:07 +08:00
Outdated
Review
Copy Link

I think we can uncomment it.

I think we can uncomment it.
张佳澈520370910044 commented 2024-05-07 15:44:01 +08:00
Outdated
Review
Copy Link

No, as long as the clang-tidy report is not empty, its return value is 1 and the executor result status is Nonzero Exit Status:

main_test.go:117: actual[1].Results[0].Comment = Unexpected executor status: Nonzero Exit Status.
        Stderr: 17855 warnings and 1 error generated.
        Error while processing /w/src/sillycode.cpp.
No, as long as the clang-tidy report is not empty, its return value is 1 and the executor result status is Nonzero Exit Status: ``` main_test.go:117: actual[1].Results[0].Comment = Unexpected executor status: Nonzero Exit Status. Stderr: 17855 warnings and 1 error generated. Error while processing /w/src/sillycode.cpp. ```
张泊明518370910136 commented 2024-05-07 15:53:48 +08:00
Outdated
Review
Copy Link

Ok, then we can assume clang-tidy do not have unexpected errors for now. clang-tidy built in rules based on Libtooling are usually very strong. An alternative approach can be checking executorResult.exitStatus if we know it will only return 0 and 1.

Ok, then we can assume clang-tidy do not have unexpected errors for now. `clang-tidy` built in rules based on Libtooling are usually very strong. An alternative approach can be checking `executorResult.exitStatus` if we know it will only return 0 and 1.
// TODO: Handle the json file (parse into markdown and delete it?) // TODO: Handle the json file (parse into markdown and delete it?)
zjc_he marked this conversation as resolved Outdated
张泊明518370910136 commented 2024-05-07 02:48:08 +08:00
Outdated
Review
Copy Link

Do we still need this file?

Do we still need this file?
张佳澈520370910044 commented 2024-05-07 09:57:00 +08:00
Outdated
Review
Copy Link

I don't know. We will need this data for drone, but I don't know where to put it for now.

I don't know. We will need this data for drone, but I don't know where to put it for now.
张泊明518370910136 commented 2024-05-07 09:58:37 +08:00
Outdated
Review
Copy Link

I think running JOJ3 will be the only step in the future. It just provides coast-to-coast experience.

I think running JOJ3 will be the only step in the future. It just provides coast-to-coast experience.
张佳澈520370910044 commented 2024-05-07 10:02:19 +08:00
Outdated
Review
Copy Link

What do you mean? JOJ3 will cover everything and we don't need drone anymore?

This data shows the details for all cases, we may need to parse them into markdown in the future, so if we don't need to save it into a file I'll just put it there and wait for future needs

What do you mean? JOJ3 will cover everything and we don't need drone anymore? This data shows the details for all cases, we may need to parse them into markdown in the future, so if we don't need to save it into a file I'll just put it there and wait for future needs
张泊明518370910136 commented 2024-05-07 10:04:43 +08:00
Outdated
Review
Copy Link

Yes. Drone/Gitea actions will only be used to trigger the running of JOJ3, and JOJ3 should be the only step. We can just leave the details in logs, so if any debug purpose, student can check the output in drone/gitea actions to see. So that we can only give simple and brief feedback and gitea issue comment, which should be enough for most cases.

Yes. Drone/Gitea actions will only be used to trigger the running of JOJ3, and JOJ3 should be the only step. We can just leave the details in logs, so if any debug purpose, student can check the output in drone/gitea actions to see. So that we can only give simple and brief feedback and gitea issue comment, which should be enough for most cases.
@ -36,6 +37,7 @@ func Parse(executorResult stage.ExecutorResult, conf Conf) stage.ParserResult {
encoder.SetIndent("", " ") encoder.SetIndent("", " ")
_ = encoder.Encode(formatted_messages) _ = encoder.Encode(formatted_messages)
// TODO: Handle unexpected errors from executor
// if executorResult.Status != stage.Status(envexec.StatusAccepted) { // if executorResult.Status != stage.Status(envexec.StatusAccepted) {
// return stage.ParserResult{ // return stage.ParserResult{
// Score: 0, // Score: 0,