feat(check): add GitLab Code Quality output

[yeshan333]

Summary

Add native GitLab Code Quality output to emmylua_check.

Changes

• Add -f gitlab with stdout and file output.
• Emit repository-relative findings with GitLab severities and deterministic fingerprints.
• Document GitLab CI usage.
• Keep the workspace clean under the current Clippy release.

Verification

• cargo test -p emmylua_check --all-features — 6 passed
• cargo clippy --workspace --all-targets --all-features -- -D warnings
• End-to-end fixtures — 3 findings for invalid code, [] for clean code

[github-actions]

Code Review: GitLab Code Quality Output Format

Summary

The change adds a new gitlab output format for GitLab Code Quality reports. Overall the implementation is clean and well-tested, but there are several issues worth addressing.

---

Issues & Recommendations

1. 🔴 Panics on I/O errors (gitlab_output_writer.rs)

The writer uses .unwrap() extensively on filesystem and serialization operations:

std::fs::create_dir_all(parent).unwrap();
Some(File::create(path).unwrap())
let report = serde_json::to_string_pretty(&self.findings).unwrap();
output.write_all(report.as_bytes()).unwrap();
output.write_all(b"\n").unwrap();

Problem: A failed directory creation, permission error, or broken pipe will crash the process with an unhelpful panic instead of a clean error message. This is especially bad for a CLI tool that may run in CI.

Recommendation: Propagate errors (e.g., return Result from new/finish, or log and exit gracefully). At minimum, use expect("...") with a descriptive message. Check how the existing json_output_writer/sarif_output_writer handle this for consistency.

---

2. 🟠 project_root fallback logic is fragile

let project_root = std::env::var_os("CI_PROJECT_DIR")
    .filter(|path| !path.is_empty())
    .map(PathBuf::from)
    .or_else(|| std::env::current_dir().ok())
    .unwrap_or(workspace);
let project_root = project_root.canonicalize().unwrap_or(project_root);

Problems:

• CI_PROJECT_DIR is only set in GitLab CI. When running locally, current_dir() is used — but the workspace argument (which the caller presumably intends as the analysis root) is only used as a last resort. This is surprising: the documented behavior says "otherwise, paths are relative to the current working directory," but the code prefers CWD over the explicitly-passed workspace. Consider whether workspace should take precedence.
• canonicalize() on Windows returns \\?\-prefixed paths, which is why strip_extended_prefix exists — but the canonicalized project_root and the (possibly non-canonicalized) file_path may not share the same prefix form, causing strip_prefix to silently fail and fall back to the absolute path. See issue #3.

---

3. 🟠 repository_relative_path may silently produce absolute paths

let relative = file_path.strip_prefix(&project_root).unwrap_or(&file_path);

Problem: If the file is outside the project root (or the prefix forms differ, e.g. one canonicalized and one not), strip_prefix fails and the function returns the absolute path. GitLab expects repository-relative paths; an absolute path like /home/runner/build/src/main.lua will not match anything and the finding will be dropped or misattributed.

Recommendation: Log a warning when the path cannot be made relative, and/or normalize both paths (canonicalize the file path too) before stripping. Consider returning Option<String> and skipping findings whose path can't be relativized.

---

4. 🟡 Fingerprint includes line number — reduces stability

fn fingerprint(check_name: &str, path: &str, begin: u32, description: &str) -> String

The README claims "stable fingerprints so GitLab can track findings between the source and target branches." However, including begin (line number) means any line shift above a finding changes its fingerprint, causing GitLab to treat it as a new issue. The test same_finding_keeps_its_fingerprint_but_path_or_line_changes_it explicitly asserts this behavior.

Recommendation: Consider excluding the line number from the fingerprint (or making it optional) so findings survive unrelated edits above them. If line-based fingerprints are intentional, document the trade-off clearly in the README.

---

5. 🟡 Severity mapping loses information

Some(DiagnosticSeverity::ERROR) => "major",
Some(DiagnosticSeverity::WARNING) => "minor",
_ => "info",

GitLab supports info, minor, major, critical, blocker. Mapping errors to major (not critical/blocker) and warnings to minor is defensible, but INFORMATION and HINT both collapse to info. This is acceptable but worth a comment explaining the rationale.

---

6. 🟡 check_name fallback is not a valid identifier

.unwrap_or_else(|| "emmylua_check".to_string());

GitLab's check_name should be a stable identifier for the rule. Using the tool name as fallback is fine, but note that diagnostics without a code will all share the same check_name — combined with the fingerprint including description, this is probably OK, but worth confirming.

---

7. 🟢 Minor: --output help text change is a behavior change

The help text changed from "only used when output_format is json" to "used by JSON-based output formats." This is accurate for the new format, but verify that sarif and github writers actually honor --output (the diff only shows Gitlab receiving output). If sarif/github ignore --output, the new wording is misleading.

---

8. 🟢 Test quality notes

• analysis_without_findings_writes_an_empty_json_array uses a timestamp-based temp filename — fine, but consider tempfile crate for robustness.
• Tests hard-code SHA-256 hashes. This is good for regression detection but makes the tests brittle if the fingerprint algorithm changes. Acceptable given the explicit intent.
• No test covers the CI_PROJECT_DIR env-var path or the "file outside project root" case (issue #3). Adding one would catch the absolute-path fallback.

---

Positive Notes

• ✅ Good use of googletest for expressive assertions.
• ✅ Fingerprint uses a null-byte separator to avoid ambiguity between concatenated fields.
• ✅ Windows extended-path prefix handling is thoughtful.
• ✅ Documentation (README + crate README) is updated consistently.

---

Priority Actions

1. Fix panics on I/O/serialization (issue #1).
2. Handle non-relativizable paths explicitly rather than emitting absolute paths (issue #3).
3. Reconsider fingerprint stability w.r.t. line numbers (issue #4).
4. Clarify project_root precedence between workspace and CWD (issue #2).

[CppCXY]

It’s a good addition. Although I don’t use GitLab, since it involves some things I’m bound to modify, I don’t plan to merge it right now—I’ll wait until I’ve finished my refactoring.