diff --git a/.clang-format b/.clang-format index f7cf3663..610dd364 100644 --- a/.clang-format +++ b/.clang-format @@ -1,6 +1,6 @@ --- Language: Cpp BasedOnStyle: Google -ColumnLimit: 100 +ColumnLimit: 80 IndentWidth: 4 TabWidth: 4 diff --git a/.github/workflows/code-quality-check.yml b/.github/workflows/code-quality-check.yml new file mode 100644 index 00000000..9ff4db66 --- /dev/null +++ b/.github/workflows/code-quality-check.yml @@ -0,0 +1,141 @@ +name: Code Quality + +permissions: + contents: read + issues: write + +on: + pull_request: + types: [opened, edited, reopened, synchronize] + paths: + - '**.py' + - '**.cpp' + - '**.h' + +jobs: + lint: + name: Code Linting + runs-on: ubuntu-latest + steps: + - name: Checkout Code + uses: actions/checkout@v3 + + - name: Set up Python + uses: actions/setup-python@v4 + with: + python-version: '3.11' + + - name: Cache pip dependencies + uses: actions/cache@v3 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements*.txt') }} + restore-keys: | + ${{ runner.os }}-pip- + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -e . + pip install pylint cpplint + + - name: Run Python Linter + id: pylint + continue-on-error: true + run: | + echo "Running pylint (configuration from pyproject.toml)..." + python -m pylint --output-format=colorized mssql_python + echo "pylint_status=$?" >> $GITHUB_ENV + + - name: Run C++ Linter + id: cpplint + continue-on-error: true + run: | + echo "Running cpplint with maximum 10 errors per file..." + + # Find C++ files excluding build directories + FILES=$(find mssql_python -name "*.cpp" -o -name "*.h" | grep -v "/build/") + if [ -z "$FILES" ]; then + echo "No C++ files found to check!" + echo "cpplint_status=0" >> $GITHUB_ENV + exit 0 + fi + + echo "Found files to check:" + echo "$FILES" + + # Process each file individually with better error handling + MAX_ERRORS=10 + FAILED_FILES="" + + for FILE in $FILES; do + echo "Checking $FILE..." + + # Run cpplint on a single file and capture output + OUTPUT=$(python -m cpplint --filter=-readability/todo --linelength=100 "$FILE" 2>&1 || true) + + # Display the output for this file + echo "$OUTPUT" + + # Extract error count more reliably + ERROR_COUNT=$(echo "$OUTPUT" | grep -o 'Total errors found: [0-9]*' | grep -o '[0-9]*' || echo "0") + + # If we couldn't extract a count, default to 0 + if [ -z "$ERROR_COUNT" ]; then + ERROR_COUNT=0 + fi + + echo "File $FILE has $ERROR_COUNT errors" + + # Check if over threshold + if [ "$ERROR_COUNT" -gt "$MAX_ERRORS" ]; then + FAILED_FILES="$FAILED_FILES\n- $FILE ($ERROR_COUNT errors)" + fi + done + + # Output results + if [ ! -z "$FAILED_FILES" ]; then + echo -e "\n⛔ The following files have more than $MAX_ERRORS errors:$FAILED_FILES" + echo "cpplint_status=1" >> $GITHUB_ENV + else + echo -e "\n✅ All files have $MAX_ERRORS or fewer errors." + echo "cpplint_status=0" >> $GITHUB_ENV + fi + + - name: Determine overall status + run: | + if [[ "${{ env.pylint_status }}" != "0" || "${{ env.cpplint_status }}" != "0" ]]; then + echo "Linting checks failed!" + exit 1 + else + echo "All linting checks passed!" + fi + + - name: Comment on PR + if: github.event_name == 'pull_request' + uses: actions/github-script@v7 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + let comment = '## Code Quality Check Results\n\n'; + + if ('${{ env.pylint_status }}' !== '0') { + comment += '⚠️ **Python linting failed** - Please check the [workflow logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.\n\n'; + } else { + comment += '✅ **Python linting passed**\n\n'; + } + + if ('${{ env.cpplint_status }}' !== '0') { + comment += '⚠️ **C++ linting failed** - Some files exceed the maximum error threshold of 10.\n\n'; + } else { + comment += '✅ **C++ linting passed**\n\n'; + } + + comment += 'See [code quality guidelines](https://github.com/microsoft/mssql-python/blob/main/CONTRIBUTING.md) for more information.'; + + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: comment + }); \ No newline at end of file diff --git a/.pre-commit-config.yml b/.pre-commit-config.yml new file mode 100644 index 00000000..31c6a00a --- /dev/null +++ b/.pre-commit-config.yml @@ -0,0 +1,15 @@ +repos: +- repo: https://github.com/pre-commit/mirrors-pylint + rev: v3.0.0a5 + hooks: + - id: pylint + +- repo: local + hooks: + - id: cpplint + name: cpplint + entry: python -m cpplint + language: python + types: [c++] + args: [--filter=-readability/todo, --linelength=100] + exclude: ^.*build/.*$ diff --git a/DEVELOPER_LINTING_GUIDE.md b/DEVELOPER_LINTING_GUIDE.md new file mode 100644 index 00000000..7359ae70 --- /dev/null +++ b/DEVELOPER_LINTING_GUIDE.md @@ -0,0 +1,293 @@ +# Developer Linting and Code Formatting Guide + +This guide helps contributors set up and use linting tools to maintain code quality standards in the mssql-python project. + +## Table of Contents + +- [Overview](#overview) +- [Prerequisites](#prerequisites) +- [Installation Instructions](#installation-instructions) + - [Windows](#windows) + - [macOS](#macos) + - [Linux](#linux) +- [Python Code Formatting](#python-code-formatting) +- [C++ Code Formatting](#c-code-formatting) +- [Pre-commit Hooks](#pre-commit-hooks) + +## Overview + +The mssql-python project uses multiple linting and formatting tools to ensure code quality: + +- **Python**: `pylint` for linting (configured in `pyproject.toml`) +- **C++**: `clang-format` for formatting, `cpplint` for linting (configured in `cpplint.cfg`) +- **Pre-commit hooks**: Already configured in `.pre-commit-config.yml` + +## Prerequisites + +Before setting up linting tools, ensure you have: + +- Python 3.8+ installed +- Git installed and configured +- A working development environment for the project + +## Quick Start (TL;DR) + +**For new contributors, this is all you need:** + +1. **Install dependencies:** + ```bash + pip install -r requirements.txt + ``` + +2. **Set up pre-commit hooks:** + ```bash + pre-commit install + ``` + +3. **Install clang-format** (see OS-specific instructions below) + +4. **Verify setup:** + ```bash + pre-commit run --all-files + ``` + +Now your code will be automatically checked before each commit! + +## Installation Instructions + +### Windows + +#### Using PowerShell (Administrator recommended for some installations) + +1. **Install Python linting tools:** + ```powershell + pip install -r requirements.txt + ``` + +2. **Install clang-format:** + + **Option A: Using Chocolatey (Recommended)** + ```powershell + # Install Chocolatey first if not installed + Set-ExecutionPolicy Bypass -Scope Process -Force; [System.Net.ServicePointManager]::SecurityProtocol = [System.Net.ServicePointManager]::SecurityProtocol -bor 3072; iex ((New-Object System.Net.WebClient).DownloadString('https://community.chocolatey.org/install.ps1')) + + # Install LLVM (includes clang-format) + choco install llvm + ``` + + **Option B: Using winget** + ```powershell + winget install LLVM.LLVM + ``` + + +3. **Verify installations:** + ```powershell + python --version + pylint --version + clang-format --version + cpplint --help + ``` + +### macOS + +1. **Install Python linting tools:** + ```bash + pip install -r requirements.txt + ``` + +2. **Install clang-format:** + ```bash + # Using Homebrew (recommended) + brew install clang-format + + # Alternative: Install full LLVM suite + brew install llvm + ``` + +3. **Verify installations:** + ```bash + python --version + pylint --version + clang-format --version + cpplint --help + ``` + +### Linux + +#### Ubuntu/Debian + +1. **Install system dependencies:** + ```bash + sudo apt update + sudo apt install clang-format python3-pip + ``` + +2. **Install Python linting tools:** + ```bash + pip install -r requirements.txt + ``` + +#### Red Hat/CentOS/Fedora + +1. **Install system dependencies:** + ```bash + # For RHEL/CentOS 8+ + sudo dnf install clang-tools-extra python3-pip + + # For older versions + sudo yum install clang python3-pip + ``` + +2. **Install Python linting tools:** + ```bash + pip install -r requirements.txt + ``` + +## Python Code Formatting + +### Using Pylint + +The project uses pylint for Python code quality checks with custom configuration in `pyproject.toml`. + +**Check a single file:** +```bash +pylint mssql_python/connection.py +``` + +**Check all Python files:** +```bash +# Windows PowerShell +Get-ChildItem -Recurse -Path mssql_python -Include *.py | ForEach-Object { pylint $_.FullName } + +# macOS/Linux +find mssql_python -name "*.py" -exec pylint {} \; +``` + +**Check specific directories:** +```bash +pylint mssql_python/ tests/ +``` + +### Project Configuration + +The project uses Pylint configuration defined in `pyproject.toml`: +- Line length: 100 characters +- Minimum score: 8.5 +- Disabled checks: fixme, no-member, too-many-arguments, etc. + +**Current project standards:** The project currently uses Pylint for both linting and enforcing formatting standards. While Black could be added for automatic formatting, the current setup relies on Pylint's formatting rules. + +## C++ Code Formatting + +### Using clang-format + +The project has a `.clang-format` configuration file with Google style guidelines. + +**Format a single file:** +```bash +clang-format -i mssql_python/pybind/ddbc_bindings.cpp +``` + +**Format all C++ files:** +```bash +# Windows PowerShell +Get-ChildItem -Recurse -Path mssql_python/pybind -Include *.cpp,*.h,*.hpp | ForEach-Object { clang-format -i $_.FullName } + +# macOS/Linux +find mssql_python/pybind -name "*.cpp" -o -name "*.h" -o -name "*.hpp" | xargs clang-format -i +``` + +**Check formatting without modifying files:** +```bash +clang-format --dry-run --Werror mssql_python/pybind/ddbc_bindings.cpp +``` + +### Using cpplint + +The project has a `cpplint.cfg` configuration file with: +- Line length: 100 characters +- Filtered out: readability/todo warnings +- Excludes: build directory + +**Check C++ code style:** +```bash +# Single file (uses project config automatically) +cpplint mssql_python/pybind/ddbc_bindings.cpp + +# All C++ files +# Windows PowerShell +Get-ChildItem -Recurse -Path mssql_python/pybind -Include *.cpp,*.h,*.hpp | ForEach-Object { cpplint $_.FullName } + +# macOS/Linux +find mssql_python/pybind -name "*.cpp" -o -name "*.h" -o -name "*.hpp" | xargs cpplint +``` + +## Pre-commit Hooks + +**Good news!** The project already has pre-commit hooks configured in `.pre-commit-config.yml`. + +The current configuration includes: +- **Pylint**: Using `pre-commit/mirrors-pylint` (v3.0.0a5) +- **cpplint**: Local hook with project-specific arguments: + - Filters out readability/todo warnings + - Line length: 100 characters + - Excludes build directory + +### Setting up pre-commit hooks: + +1. **Install pre-commit (already in requirements.txt):** + ```bash + pip install pre-commit + ``` + +2. **Install the hooks:** + ```bash + pre-commit install + ``` + +3. **Run hooks on all files (optional):** + ```bash + pre-commit run --all-files + ``` + +4. **Run specific hooks:** + ```bash + # Run only pylint + pre-commit run pylint + + # Run only cpplint + pre-commit run cpplint + ``` + +### How it works: +- Hooks run automatically on `git commit` +- Only files being committed are checked +- Commit is blocked if linting fails +- You can bypass with `git commit --no-verify` (not recommended) + +## Best Practices + +1. **Always run linting before committing:** + - Set up pre-commit hooks + - Run manual checks for critical changes + +2. **Fix linting issues promptly:** + - Don't accumulate technical debt + - Address issues in the same PR that introduces them + +3. **Understand the rules:** + - Read pylint and cpplint documentation + - Understand why certain patterns are discouraged + +4. **Team consistency:** + - Follow the project's existing style + - Discuss style changes in team meetings + +## Additional Resources + +- [Pylint Documentation](https://pylint.pycqa.org/en/latest/) +- [Black Documentation](https://black.readthedocs.io/en/stable/) +- [Clang-Format Documentation](https://clang.llvm.org/docs/ClangFormat.html) +- [cpplint Style Guide](https://google.github.io/styleguide/cppguide.html) +- [Pre-commit Documentation](https://pre-commit.com/) diff --git a/cpplint.cfg b/cpplint.cfg new file mode 100644 index 00000000..1985d7ba --- /dev/null +++ b/cpplint.cfg @@ -0,0 +1,4 @@ +set noparent +filter=-readability/todo +linelength=100 +exclude_files=build diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 00000000..edb032c3 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,11 @@ +[tool.pylint] +disable = [ + "fixme", + "no-member", + "too-many-arguments", + "too-many-positional-arguments", + "invalid-name", + "useless-parent-delegation" +] +fail-under = 8.5 +max-line-length = 100 diff --git a/requirements.txt b/requirements.txt index 5abf13dc..99d242ce 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,4 +4,7 @@ pybind11 coverage unittest-xml-reporting setuptools -psutil \ No newline at end of file +psutil +pylint +cpplint +pre-commit \ No newline at end of file