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 +106
6 changed files with 106 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit be21a16293 - Show all commits

1
internal/executors/all.go
View File

@ -1,6 +1,7 @@
package executors
import (
_ "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/executors/clang_tidy"
_ "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/executors/dummy"
_ "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/executors/sandbox"
)

29
internal/executors/clang_tidy/executor.go Normal file
View File

@ -0,0 +1,29 @@
package clang_tidy
import (
"focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/stage"
"github.com/criyle/go-judge/envexec"
)
type ClangTidy struct{}
func (e *ClangTidy) Run(cmds []stage.Cmd) ([]stage.ExecutorResult, error) {
var res []stage.ExecutorResult
for range cmds {
res = append(res, stage.ExecutorResult{
Status: stage.Status(envexec.StatusInvalid),
ExitStatus: 0,
Error: "I'm a dummy",
Time: 0,
Memory: 0,
RunTime: 0,
Files: map[string]string{},
FileIDs: map[string]string{},
})
}
zjc_he marked this conversation as resolved Outdated
张泊明518370910136 commented 2024-05-03 06:39:01 +08:00
Outdated
Review
Copy Link

I do not think we need this executor. We can just run everything inside the sandbox.

I do not think we need this executor. We can just run everything inside the sandbox.
return res, nil
}
func (e *ClangTidy) Cleanup() error {
return nil
}

9
internal/executors/clang_tidy/meta.go Normal file
View File

@ -0,0 +1,9 @@
package clang_tidy
import "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/stage"
var name = "clang-tidy"
func init() {
stage.RegisterExecutor(name, &ClangTidy{})
}

1
internal/parsers/all.go
View File

@ -1,6 +1,7 @@
package parsers
import (
_ "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/parsers/clang_tidy"
_ "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/parsers/diff"
_ "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/parsers/dummy"
_ "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/parsers/resultstatus"

9
internal/parsers/clang_tidy/meta.go Normal file
View File

@ -0,0 +1,9 @@
package clang_tidy
import "focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/stage"
var name = "clang-tidy"
func init() {
stage.RegisterParser(name, &ClangTidy{})
}

57
internal/parsers/clang_tidy/parser.go Normal file
View File

@ -0,0 +1,57 @@
package clang_tidy
import (
"encoding/json"
"fmt"
"focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/internal/stage"
"focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/pkg/dummy"
"github.com/criyle/go-judge/envexec"
)
type Conf struct {
Score int
Comment string
}
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
type ClangTidy struct{}
func Parse(executorResult stage.ExecutorResult, conf Conf) stage.ParserResult {
stdout := executorResult.Files["stdout"]
stderr := executorResult.Files["stderr"]
if executorResult.Status != stage.Status(envexec.StatusAccepted) {
return stage.ParserResult{
Score: 0,
Comment: fmt.Sprintf(
"Unexpected executor status: %s.\nStderr: %s",
executorResult.Status, stderr,
),
}
}
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.
var dummyResult dummy.Result
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.
err := json.Unmarshal([]byte(stdout), &dummyResult)
if err != nil {
return stage.ParserResult{
Score: 0,
Comment: fmt.Sprintf("Failed to parse result: %s", err),
}
}
return stage.ParserResult{
Score: dummyResult.Score + conf.Score,
Comment: dummyResult.Comment + conf.Comment,
}
}
func (*ClangTidy) Run(results []stage.ExecutorResult, confAny any) (
[]stage.ParserResult, bool, error,
) {
conf, err := stage.DecodeConf[Conf](confAny)
if err != nil {
return nil, true, err
}
var res []stage.ParserResult
for _, result := range results {
zjc_he marked this conversation as resolved Outdated
张泊明518370910136 commented 2024-05-03 06:42:19 +08:00
Outdated
Review
Copy Link

We just put all the output of the parser in Comment.

We just put all the output of the parser in Comment.
张佳澈520370910044 commented 2024-05-03 10:05:17 +08:00
Outdated
Review
Copy Link

Do you mean the json file which shows the whole detail of outputs? That might be really really long

Do you mean the json file which shows the whole detail of outputs? That might be really really long
张泊明518370910136 commented 2024-05-05 03:13:38 +08:00
Outdated
Review
Copy Link

The comment just gives human readable results. We may log the actual output for further details, but the comment will be replied in gitea issues. So we can just keep it short.

The comment just gives human readable results. We may log the actual output for further details, but the comment will be replied in gitea issues. So we can just keep it short.
张佳澈520370910044 commented 2024-05-05 21:40:54 +08:00
Outdated
Review
Copy Link

A current version of comment:

Test results summary

  1. codequality-unchecked-malloc-result: 0
  2. codequality-no-global-variables: 0
  3. codequality-no-header-guard: 0
  4. codequality-no-fflush-stdin: 0
  5. readability-function-size: 0
  6. readability-identifier-naming: 0
  7. readability-redundant: 0
  8. readability-misleading-indentation: 0
  9. readability-misplaced-array-index: 0
  10. cppcoreguidelines-init-variables: 1
  11. bugprone-suspicious-string-compare: 0
  12. google-global-names-in-headers: 0
  13. clang-diagnostic: 1
  14. clang-analyzer: 0
  15. misc: 8
  16. performance: 4
  17. others: 187

I followed the version in ve482 issue

A current version of comment: ### Test results summary 1. codequality-unchecked-malloc-result: 0 2. codequality-no-global-variables: 0 3. codequality-no-header-guard: 0 4. codequality-no-fflush-stdin: 0 5. readability-function-size: 0 6. readability-identifier-naming: 0 7. readability-redundant: 0 8. readability-misleading-indentation: 0 9. readability-misplaced-array-index: 0 10. cppcoreguidelines-init-variables: 1 11. bugprone-suspicious-string-compare: 0 12. google-global-names-in-headers: 0 13. clang-diagnostic: 1 14. clang-analyzer: 0 15. misc: 8 16. performance: 4 17. others: 187 I followed the version in ve482 issue
res = append(res, Parse(result, *conf))
}
return res, false, nil
}