Skip to content

Commit 5dcad74

Browse files
thomaslaurensonthomaslaurenson
authored
Refactor/linting (#155)
* Added clang-tidy configuration * Added clang-format configuration * Code base changes to meet linting rules * Added lint workflow * Updated Makefile * Updated contributing docs
1 parent 7b458f5 commit 5dcad74

20 files changed

Lines changed: 936 additions & 621 deletions

‎.clang-format‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
BasedOnStyle: Google
2+
PointerAlignment: Right
3+
DerivePointerAlignment: false
4+
ColumnLimit: 100
5+
IndentWidth: 4
6+
AccessModifierOffset: -4
7+
IncludeBlocks: Regroup
8+
IncludeIsMainRegex: '([-_]test)?$'
9+
IncludeCategories:
10+
- Regex: '^<(StormLib\.h|CLI/)'
11+
Priority: 2
12+
- Regex: '^<'
13+
Priority: 1
14+
- Regex: '^"'
15+
Priority: 3
16+
ReflowComments: true
17+
BreakBeforeBraces: Attach
18+
Cpp11BracedListStyle: true
19+
AllowShortFunctionsOnASingleLine: InlineOnly
20+
SortIncludes: true

‎.clang-tidy‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
Checks: >
2+
clang-analyzer-core.*,
3+
clang-analyzer-cplusplus.*,
4+
clang-analyzer-deadcode.*,
5+
modernize-use-nullptr,
6+
bugprone-*,
7+
-bugprone-easily-swappable-parameters,
8+
-bugprone-exception-escape,
9+
-bugprone-narrowing-conversions,
10+
cppcoreguidelines-no-malloc,
11+
-cppcoreguidelines-owning-memory,
12+
performance-unnecessary-value-param,
13+
readability-inconsistent-declaration-parameter-name,
14+
readability-container-size-empty
15+
WarningsAsErrors: "*"
16+
FormatStyle: file

‎.github/workflows/lint.yml‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
name: Lint
2+
3+
on:
4+
workflow_call
5+
6+
jobs:
7+
lint_cpp:
8+
runs-on: ubuntu-24.04
9+
steps:
10+
- name: Check out repository code
11+
uses: actions/checkout@v6
12+
with:
13+
submodules: true
14+
15+
- name: Install clang tools
16+
run: make install_clang_tools
17+
18+
- name: Check formatting
19+
run: make lint
20+
21+
- name: Generate compile_commands.json
22+
run: make build_lint/compile_commands.json
23+
24+
- name: Run clang-tidy
25+
run: make lint_cpp

‎.github/workflows/main.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,16 @@ permissions:
1616
jobs:
1717
build:
1818
uses: ./.github/workflows/build.yml
19+
lint:
20+
uses: ./.github/workflows/lint.yml
21+
needs: build
1922
test:
2023
uses: ./.github/workflows/test.yml
2124
needs: build
2225
prerelease:
2326
uses: ./.github/workflows/prerelease.yml
2427
needs:
2528
- build
29+
- lint
2630
- test
2731
secrets: inherit

‎.github/workflows/pr.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ permissions:
99
jobs:
1010
build:
1111
uses: ./.github/workflows/build.yml
12+
lint:
13+
uses: ./.github/workflows/lint.yml
14+
needs: build
1215
test:
1316
uses: ./.github/workflows/test.yml
1417
needs: build

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# CUSTOM
2+
/build_lint
23
/test/data
34

45
# C++

‎CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ if(NOT CMAKE_BUILD_TYPE)
1515
endif()
1616

1717
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
18+
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
1819

1920
# Static linking configuration
2021
if(BUILD_STATIC)

‎CONTRIBUTING.md‎

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,40 @@ If you are unsure whether a feature fits the project, or whether an existing too
88

99
**mpqcli follows the Unix philosophy.** The tool is designed to do one thing well and to compose with other tools via pipes and redirection. If you find yourself wanting to add functionality that could be handled by a separate tool — for example, sorting the output of `list` — the right answer is usually to pipe the output to that tool rather than adding it here.
1010

11+
## Prerequisites and Setup
12+
13+
Clone the repository and initialise submodules:
14+
15+
```
16+
git clone https://github.com/TheGrayDot/mpqcli.git
17+
cd mpqcli
18+
git submodule update --init --recursive
19+
```
20+
21+
Install the clang lint tools:
22+
23+
```
24+
make setup
25+
```
26+
27+
## Makefile Reference
28+
29+
Run `make help` to list all available targets. Common ones:
30+
31+
| Target | Description |
32+
|---|---|
33+
| `make setup` | Install clang-format and clang-tidy via apt |
34+
| `make build_linux` | Build for Linux using cmake |
35+
| `make build_windows` | Build for Windows using cmake |
36+
| `make build_clean` | Remove the cmake build directory |
37+
| `make test_create_venv` | Create Python venv and install test dependencies (first-time only) |
38+
| `make test_mpqcli` | Run the pytest test suite |
39+
| `make lint` | Run all C++ linters (clang-format + clang-tidy) |
40+
| `make lint_format` | Check formatting only (dry run) |
41+
| `make lint_format_fix` | Auto-fix formatting in-place |
42+
| `make lint_cpp` | Run clang-tidy static analysis |
43+
| `make clean` | Remove all build and test artifacts |
44+
1145
## Requirements for a Pull Request
1246

1347
### 1. Builds on your platform
@@ -36,12 +70,54 @@ All tests must pass without errors.
3670

3771
If your change adds or modifies user-facing functionality — such as a new subcommand flag or a change in output format — please include a corresponding test in the `test/` directory. The existing test files (`test_list.py`, `test_add.py`, etc.) are good references for the test style and fixtures used.
3872

39-
### 4. Match the existing code style
73+
### 4. Linting must pass
74+
75+
All C++ code is formatted with clang-format and analysed with clang-tidy. Run the full suite before submitting:
76+
77+
```
78+
make lint
79+
```
4080

41-
There is no enforced formatter. Write C++ that looks consistent with the surrounding code, and Python tests that follow the style of the existing test files.
81+
If there are formatting violations, auto-fix them with:
82+
83+
```
84+
make lint_format_fix
85+
```
86+
87+
Then re-run `make lint` to confirm everything passes.
88+
89+
### 5. Match the existing code style
90+
91+
C++ formatting is enforced by `.clang-format` (Google style base). Static analysis is enforced by `.clang-tidy`. Both configs live in the repo root. Python tests should follow the style of the existing test files.
92+
93+
#### Suppression policy
94+
95+
Suppressions are occasionally necessary for third-party code or intentional patterns. When suppressing a clang-tidy warning:
96+
97+
- Use `// NOLINT(check-name)` with the specific check name — bare `// NOLINT` is not acceptable
98+
- Every suppression must have a comment explaining why it is justified
99+
100+
```cpp
101+
// NOLINT(bugprone-easily-swappable-parameters): parameters validated by CLI11
102+
```
103+
104+
#### Disabling clang-format locally
105+
106+
Use `// clang-format off` / `// clang-format on` only when the default formatting genuinely hurts readability (e.g. column-aligned tables). Add a brief comment explaining the intent:
107+
108+
```cpp
109+
// clang-format off: preserve column-aligned flag-to-char mappings for readability
110+
if (flags & MPQ_FILE_IMPLODE) result += 'i';
111+
if (flags & MPQ_FILE_COMPRESS) result += 'c';
112+
// clang-format on
113+
```
42114

43115
## Workflow Summary
44116

45117
1. Fork the repository and create a branch for your change
46-
2. Make your changes and verify they build and all tests pass
47-
3. Open a pull request with a clear description of what was changed and why
118+
2. Run `git submodule update --init --recursive` after cloning
119+
3. Run `make install_clang_tools` to install lint dependencies
120+
4. Make your changes and verify they build: `make build_linux`
121+
5. Run `make lint` and fix any issues
122+
6. Run `make test_mpqcli` and confirm all tests pass
123+
7. Open a pull request with a clear description of what was changed and why

‎Makefile‎

Lines changed: 80 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,89 +1,149 @@
11
CMAKE_BUILD_TYPE := Release
22
BUILD_MPQCLI := ON
3+
CLANG_VERSION := 18
34
VERSION := $(shell awk '/project\(MPQCLI VERSION/ {gsub(/\)/, "", $$3); print $$3}' CMakeLists.txt)
45
README := README.md
56
PACKAGE_URL := https://github.com/TheGrayDot/mpqcli/pkgs/container/mpqcli
67

8+
GCC_INSTALL_DIR := $(shell dirname "$(shell gcc -print-libgcc-file-name)")
9+
710
.PHONY: help \
8-
build_linux build_windows build_clean \
11+
setup \
12+
build_linux build_windows build_clean build_lint_clean \
913
docker_musl_build docker_musl_run docker_glibc_build docker_glibc_run \
1014
test_create_venv test_mpqcli test_clean test_lint \
15+
lint_format lint_format_fix lint_cpp lint \
1116
clean \
1217
bump_stormlib bump_cli11 bump_submodules \
1318
fetch_downloads tag_release
1419

15-
help: ## Show this help menu
16-
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n\nTargets:\n"} \
17-
/^[a-zA-Z0-9_-]+:.*?##/ { printf " \033[36m%-22s\033[0m %s\n", $$1, $$2 }' $(MAKEFILE_LIST)
20+
## Show this help menu
21+
help:
22+
@awk 'BEGIN {FS = ":"; printf "\nUsage:\n make \033[36m<target>\033[0m\n\nTargets:\n"} \
23+
/^## / {desc = substr($$0, 4); next} \
24+
/^[a-zA-Z0-9_-]+:/ {if (desc) printf " \033[36m%-22s\033[0m %s\n", $$1, desc; desc = ""; next} \
25+
{desc = ""}' $(MAKEFILE_LIST)
26+
27+
## Install clang lint dependencies
28+
install_clang_tools:
29+
sudo apt-get install -y clang-format-$(CLANG_VERSION) clang-tidy-$(CLANG_VERSION)
1830

1931
# BUILD
20-
build_linux: ## Build for Linux using cmake
32+
## Build for Linux using cmake
33+
build_linux:
2134
cmake -B build \
2235
-DCMAKE_BUILD_TYPE=$(CMAKE_BUILD_TYPE) \
2336
-DBUILD_MPQCLI=$(BUILD_MPQCLI)
2437
cmake --build build
2538

26-
build_windows: ## Build for Windows using cmake
39+
## Build for Windows using cmake
40+
build_windows:
2741
cmake -B build \
2842
-DCMAKE_BUILD_TYPE=$(CMAKE_BUILD_TYPE) \
2943
-DBUILD_MPQCLI=$(BUILD_MPQCLI)
3044
cmake --build build --config $(CMAKE_BUILD_TYPE)
3145

32-
build_clean: ## Remove cmake build directory
46+
## Remove cmake build directory
47+
build_clean:
3348
rm -rf build
3449

50+
## Generate compile_commands.json for clang-tidy
51+
build_lint/compile_commands.json: CMakeLists.txt src/CMakeLists.txt
52+
cmake -B build_lint \
53+
-DCMAKE_BUILD_TYPE=Debug \
54+
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
55+
-DBUILD_MPQCLI=ON \
56+
-DCMAKE_CXX_COMPILER=clang++-$(CLANG_VERSION) \
57+
-DCMAKE_CXX_FLAGS="--gcc-install-dir=$(GCC_INSTALL_DIR)"
58+
59+
## Remove cmake lint build directory
60+
build_lint_clean:
61+
rm -rf build_lint
62+
3563
# DOCKER
36-
docker_musl_build: ## Build Docker image using musl
64+
## Build Docker image using musl
65+
docker_musl_build:
3766
docker build -t mpqcli:$(VERSION) -f Dockerfile.musl .
3867

39-
docker_musl_run: ## Run the musl Docker image
68+
## Run the musl Docker image
69+
docker_musl_run:
4070
@docker run -it mpqcli:$(VERSION) version
4171

42-
docker_glibc_build: ## Build Docker image using glibc
72+
## Build Docker image using glibc
73+
docker_glibc_build:
4374
docker build -t mpqcli:$(VERSION) -f Dockerfile.glibc .
4475

45-
docker_glibc_run: ## Run the glibc Docker image
76+
## Run the glibc Docker image
77+
docker_glibc_run:
4678
@docker run -it mpqcli:$(VERSION) version
4779

4880
# TEST
49-
test_create_venv: ## Create Python venv and install test dependencies
81+
## Create Python venv and install test dependencies
82+
test_create_venv:
5083
python3 -m venv ./.venv
5184
. ./.venv/bin/activate && \
5285
pip3 install -r test/requirements.txt
5386

54-
test_mpqcli: ## Run pytest test suite
87+
## Run pytest test suite
88+
test_mpqcli:
5589
. ./.venv/bin/activate && \
5690
python3 -m pytest test -s
5791

58-
test_clean: ## Remove test data directory
92+
## Remove test data directory
93+
test_clean:
5994
rm -rf test/data
6095

61-
test_lint: ## Run ruff linter on test directory
96+
## Run ruff linter on test directory
97+
test_lint:
6298
. ./.venv/bin/activate && \
6399
ruff check ./test
64100

101+
# LINT
102+
## Check C++ formatting with clang-format
103+
lint_format:
104+
find src \( -name "*.cpp" -o -name "*.h" \) \
105+
| xargs clang-format-$(CLANG_VERSION) --dry-run --Werror
106+
107+
## Auto-fix C++ formatting with clang-format
108+
lint_format_fix:
109+
find src \( -name "*.cpp" -o -name "*.h" \) \
110+
| xargs clang-format-$(CLANG_VERSION) -i
111+
112+
## Run clang-tidy static analysis
113+
lint_cpp: build_lint/compile_commands.json
114+
clang-tidy-$(CLANG_VERSION) \
115+
--quiet -p build_lint --header-filter="$(CURDIR)/src/.*" src/*.cpp
116+
117+
## Run all C++ linters
118+
lint: lint_format lint_cpp
119+
65120
# CLEAN
66-
clean: build_clean test_clean ## Remove all build and test artifacts
121+
## Remove all build and test artifacts
122+
clean: build_clean build_lint_clean test_clean
67123

68124
# SUBMODULES
69-
bump_stormlib: ## Bump StormLib submodule to latest remote
125+
## Bump StormLib submodule to latest remote
126+
bump_stormlib:
70127
@read -rp "[*] Bump StormLib? (y/N) " yn; \
71128
case $$yn in \
72129
[yY] ) git submodule update --init --remote extern/StormLib;; \
73130
* ) echo "[*] Skipping...";; \
74131
esac
75132

76-
bump_cli11: ## Bump CLI11 submodule to latest remote
133+
## Bump CLI11 submodule to latest remote
134+
bump_cli11:
77135
@read -rp "[*] Bump CLI11? (y/N) " yn; \
78136
case $$yn in \
79137
[yY] ) git submodule update --init --remote extern/CLI11;; \
80138
* ) echo "[*] Skipping...";; \
81139
esac
82140

83-
bump_submodules: bump_stormlib bump_cli11 ## Bump all submodules to latest remote
141+
## Bump all submodules to latest remote
142+
bump_submodules: bump_stormlib bump_cli11
84143

85144
# RELEASE
86-
fetch_downloads: ## Fetch package downloads and update README.md badge
145+
## Fetch package downloads and update README.md badge
146+
fetch_downloads:
87147
@DOWNLOADS=$$(curl -s "$(PACKAGE_URL)" \
88148
| grep -A2 "Total downloads" \
89149
| grep -o '<h3 title="[0-9]*">[0-9]*</h3>' \
@@ -92,12 +152,3 @@ fetch_downloads: ## Fetch package downloads and update README.md badge
92152
| head -1); \
93153
sed -i "s/package_downloads-[0-9]*-green/package_downloads-$$DOWNLOADS-green/" $(README); \
94154
echo "[*] Updated package downloads badge: $$DOWNLOADS"
95-
96-
tag_release: ## Tag and push the current project version
97-
@echo "[*] Current version: v$(VERSION)"
98-
@read -rp "[*] Tag and Release? (y/N) " yn; \
99-
case $$yn in \
100-
[yY] ) git tag "v$(VERSION)" && git push --tags && echo "[*] Tagged and pushed v$(VERSION)";; \
101-
[nN] ) echo "[*] Exiting...";; \
102-
* ) echo "[*] Invalid response... Exiting"; exit 1;; \
103-
esac

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
![Release Version](https://img.shields.io/github/v/release/TheGrayDot/mpqcli?style=flat)
66

7-
![Release downloads](https://img.shields.io/github/downloads/thegraydot/mpqcli/total?label=release_downloads) ![Package downloads](https://img.shields.io/badge/package_downloads-353-green)
7+
![Release downloads](https://img.shields.io/github/downloads/thegraydot/mpqcli/total?label=release_downloads) ![Package downloads](https://img.shields.io/badge/package_downloads-497-green)
88

99
A command-line tool to create, add, remove, list, extract, read, and verify MPQ archives using the [StormLib library](https://github.com/ladislav-zezula/StormLib).
1010

0 commit comments

Comments
 (0)