feat(linter): add linter service framework #907

HamdaanAliQuatil · 2025-09-14T13:15:43Z

Have you followed the guidelines for contributing?
Have you signed the CLA?
Have you successfully run make lint && make test?
Have you added an entry to the changelog (docs/reference/changelog.rst)?

Closes #782
This PR introduces a shared Linter Service to craft-application so downstream apps (Snapcraft, Charmcraft) can add and run linters consistently.

The design of the Linter Service is as discussed in #782

lengau

Thanks! I've done an initial review with a few things that need changing.

craft_application/commands/other.py

craft_application/services/linter.py

craft_application/lint/types.py

craft_application/services/linter.py

medubelko · 2025-09-22T16:39:59Z

Hi @HamdaanAliQuatil, thanks for taking this on. :)

I'm holding off my documentation review until the implementation is complete. If it looks like the implementation is a ways off, it might be better to split the documentation portion into its own PR afterward.

lengau

Looking good so far!

Please add a couple of linters to testcraft and a spread test that tests for the app outputting the expected values.

A good pre linter would be a recommended field (e.g. version) being missing. A good post linter would be that the artifact is empty other than the generated metadata.yaml file.

craft_application/commands/lint.py

HamdaanAliQuatil · 2025-10-01T09:41:04Z

@lengau spread tests in snap-tests (google:) were failing due to path discrepancy. I've updated the paths. Can you please retrigger workflows.

lengau · 2025-10-10T20:14:13Z

@HamdaanAliQuatil I've merged from main and re-triggered the workflows. Please address the failure with the lint command in testcraft, but otherwise I'm pretty happy with this. Thanks!

HamdaanAliQuatil · 2025-10-22T09:36:12Z

@lengau do we have any updates on this. Can you please re-trigger the workflows for me.

craft_application/lint/types.py

docs/reference/services/linter.rst

craft_application/services/linter.py

tests/integration/services/test_linter.py

tests/unit/commands/test_lint.py

tests/unit/services/test_linter_service.py

HamdaanAliQuatil · 2025-11-10T15:28:07Z

@bepri bringing this up again :) can you please retrigger the tests if everything looks good now

bepri

Thanks! I'm down to just nitpicks now, personally.

craft_application/services/linter.py

craft_application/util/logging.py

craft_application/commands/lint.py

Copilot

Pull Request Overview

This PR introduces a shared Linter Service framework to craft-application, enabling downstream applications (Snapcraft, Charmcraft) to add and run linters consistently across pre- and post-build stages.

Implements a service-based linter framework with class-level registration and central ignore configuration
Adds a public lint command with CLI support for ignore rules and custom ignore files
Includes example linters in Testcraft demonstrating pre-build and post-build linting

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
craft_application/lint/_types.py	Defines core types (Stage, Severity, ExitCode, LintContext, LinterIssue, IgnoreSpec) and the should_ignore function
craft_application/lint/base.py	Implements AbstractLinter base class with validation for name and stage attributes
craft_application/lint/init.py	Public API exports for the lint module
craft_application/lint/linters/init.py	Placeholder namespace for built-in linters
craft_application/services/linter.py	Core LinterService implementation with registration, ignore config loading, and execution logic
craft_application/services/init.py	Exports LinterService
craft_application/services/service_factory.py	Adds linter and package service registrations with type hints
craft_application/commands/lint.py	Implements the lint CLI command with ignore rule parsing
craft_application/commands/init.py	Exports LintCommand
craft_application/util/logging.py	Updates print_error default from report_error to emit.error
testcraft/services/linter.py	Testcraft-specific linter implementations (MissingVersionLinter, EmptyArtifactLinter) and service subclass
testcraft/services/init.py	Registers TestcraftLinterService
testcraft/cli.py	Adds LintCommand to Testcraft CLI
tests/unit/services/test_linter_service.py	Unit tests for linter service registration, ignore rules, and hooks
tests/unit/commands/test_lint.py	Unit tests for lint command and CLI ignore config parsing
tests/integration/services/test_linter.py	Integration test for linter service with ignore config workflow
tests/spread/testcraft/lint/task.yaml	Spread test validating lint command output for pre and post stages
tests/spread/testcraft/lint/project-missing-version.yaml	Test fixture for pre-stage linting
tests/spread/testcraft/lint/project-with-version.yaml	Test fixture for post-stage linting
docs/reference/services/linter.rst	Reference documentation for the linter service API
docs/reference/services/index.rst	Adds linter service to index
docs/how-to-guides/add-a-linter.rst	How-to guide for adding new linters
docs/how-to-guides/index.rst	Adds linter guide to index

Comments suppressed due to low confidence (1)

craft_application/util/logging.py:58

The docstring mentions "Emitter.report_error" but the code now uses craft_cli.emit.error (line 51). The docstring should be updated to match the actual implementation and reference emit.error instead of Emitter.report_error.

    """Handle a runtime error by transforming and printing it, then return the appropriate retcode.

    The printing behavior can be customized by passing a different `print_error`, otherwise
    `Emitter.report_error` is used.
    """

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/unit/services/test_linter_service.py

…an API; improve tests and docs

HamdaanAliQuatil · 2025-11-18T21:51:07Z

@bepri could you please retrigger the tests

bepri · 2025-11-19T14:59:40Z

Please refrain from force-pushes, it messes up GH's tracking of what files/changes we've already reviewed and thus slows down the review process. This PR is going to be squash-merged anyways, so the commit history doesn't matter :) Thanks!

It looks like in the force push, a couple of my more recent suggestions you applied were dropped again:

HamdaanAliQuatil · 2025-11-20T15:58:05Z

noted 🫡 @bepri I've reverted the missed changes and fixed the spread test failure to ensure the Testcraft linter service correctly extracts .testcraft tarballs even when no artifact directories are specified. This allows to fire TC100 again, and I've added a unit test to cover this behavior.

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

craft_application/services/linter.py:1

Using tarfile.extractall() without filters is unsafe and can lead to path traversal vulnerabilities. Use tar.extractall(path=tmp_path, filter='data') to prevent extraction of files outside the target directory.

# This file is part of craft-application.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

testcraft/services/linter.py

bepri

I'm approving this because I'm happy with the current functionality, but I have just a couple more minor nitpicks about the code that I'd like to be added before this is merged. Thanks!

@medubelko @lengau I think this is ready for your review.

craft_application/commands/lint.py

craft_application/services/linter.py

medubelko

Hi @HamdaanAliQuatil, I'm a technical author at Canonical who looks at design and documentation.

I did some testing, and I have some questions about the UX and a suggestion for the CLI messages. Let me know what you think.

craft_application/commands/lint.py

medubelko · 2025-12-18T00:30:49Z

craft_application/commands/lint.py

+        if issues:
+            emit.message("lint results:")
+            for linter_name, linter_issues in linter.issues_by_linter.items():
+                emit.message(f"{linter_name}:")
+                for issue in linter_issues:
+                    location = f" ({issue.filename})" if issue.filename else ""
+                    emit.message(
+                        f"  - [{issue.severity.name}] {issue.id}: {issue.message}{location}"
+                    )


From some initial testing, it seems that YAML syntax errors never reach this code. Instead, YamlError (I believe) interrupts. Is this expected?

Source:

name: testcraft-test version: "1.0" base: ubuntu@24.04 platforms: platform-independent: build-on: [amd64, arm64, ppc64el, s390x, riscv64] build-for: [all] parts: my-test: plugin: nil parts:

Result:

$ testcraft lint error parsing '(unknown)': found duplicate key 'parts' Detailed information: while constructing a mapping found duplicate key 'parts' in "<unicode string>", line 1, column 1: name: testcraft-test ^ Recommended resolution: Ensure (unknown) contains valid YAML

Yes, this is expected.

Invalid YAML is treated as a fatal parsing error, so the lint results block isn’t reached. This is expected for now, unless we decide to convert YAML errors into lint issues explicitly.

HamdaanAliQuatil · 2025-12-27T19:01:12Z

@lengau @bepri @medubelko updated the PR to incorporate your suggestions 🙌

HamdaanAliQuatil requested review from a team and medubelko as code owners September 14, 2025 13:32

lengau requested changes Sep 15, 2025

View reviewed changes

lengau reviewed Sep 25, 2025

View reviewed changes

craft_application/commands/lint.py Outdated Show resolved Hide resolved

HamdaanAliQuatil force-pushed the feat/linter-service branch from 32e3a0e to 8fdcaf9 Compare September 26, 2025 03:28

HamdaanAliQuatil requested a review from lengau September 27, 2025 17:37

lengau requested review from bepri and tigarmo October 10, 2025 20:14

HamdaanAliQuatil requested review from a team as code owners October 18, 2025 07:32

bepri reviewed Oct 22, 2025

View reviewed changes

HamdaanAliQuatil requested a review from bepri October 31, 2025 00:00

bepri reviewed Nov 17, 2025

View reviewed changes

craft_application/services/linter.py Outdated Show resolved Hide resolved

craft_application/util/logging.py Outdated Show resolved Hide resolved

craft_application/commands/lint.py Outdated Show resolved Hide resolved

Copilot AI review requested due to automatic review settings November 17, 2025 18:09

Copilot started reviewing on behalf of HamdaanAliQuatil November 17, 2025 18:10 View session

Copilot finished reviewing on behalf of HamdaanAliQuatil November 17, 2025 18:13

Copilot AI reviewed Nov 17, 2025

View reviewed changes

tests/unit/services/test_linter_service.py Show resolved Hide resolved

tests/unit/services/test_linter_service.py Show resolved Hide resolved

tests/unit/services/test_linter_service.py Show resolved Hide resolved

HamdaanAliQuatil added 8 commits November 19, 2025 01:23

feat(linter): add linter service framework

dafd60c

docs(linter): add documentation for linter

63e9920

test(linter): add integration test for linter

f2b5455

feat(linter): add lint report per linter

db68d12

feat(testcraft): add sample linters and spread test

fd59c85

test(spread): copy yaml from suite cwd in lint task

a9d0ad8

fix(lint): configure project before running post-stage lint

77709e1

refactor(linter): centralize ignore handling, CLI parsing; expose cle…

3c97adf

…an API; improve tests and docs

HamdaanAliQuatil force-pushed the feat/linter-service branch from 45c057e to 3c97adf Compare November 18, 2025 19:56

test(linter-service): seed dummy linter via autouse fixture

fb804e1

fix(testcraft): unpack packed artifacts before post-stage lint

3f7d420

Merge branch 'main' into feat/linter-service

bd78a02

lengau requested a review from Copilot December 6, 2025 06:43

Copilot AI reviewed Dec 6, 2025

View reviewed changes

lengau requested a review from Copilot December 6, 2025 07:51

Copilot AI reviewed Dec 6, 2025

View reviewed changes

testcraft/services/linter.py Show resolved Hide resolved

bepri approved these changes Dec 9, 2025

View reviewed changes

craft_application/commands/lint.py Outdated Show resolved Hide resolved

craft_application/commands/lint.py Outdated Show resolved Hide resolved

craft_application/commands/lint.py Outdated Show resolved Hide resolved

craft_application/services/linter.py Outdated Show resolved Hide resolved

bepri removed request for a team and tigarmo December 9, 2025 15:08

medubelko reviewed Dec 18, 2025

View reviewed changes

refactor(lint): tighten arg parsing and typing casts

e07c6f1

Merge branch 'main' into feat/linter-service

698d6be

feat(linter): add linter service framework #907

Are you sure you want to change the base?

feat(linter): add linter service framework #907

Uh oh!

Conversation

HamdaanAliQuatil commented Sep 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lengau left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

medubelko commented Sep 22, 2025

Uh oh!

lengau left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HamdaanAliQuatil commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lengau commented Oct 10, 2025

Uh oh!

HamdaanAliQuatil commented Oct 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HamdaanAliQuatil commented Nov 10, 2025

Uh oh!

bepri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HamdaanAliQuatil commented Nov 18, 2025

Uh oh!

bepri commented Nov 19, 2025

Uh oh!

HamdaanAliQuatil commented Nov 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

bepri left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

medubelko left a comment

HamdaanAliQuatil commented Sep 14, 2025 •

edited

Loading

HamdaanAliQuatil commented Oct 1, 2025 •

edited

Loading

bepri left a comment •

edited

Loading

medubelko Dec 18, 2025 •

edited

Loading

HamdaanAliQuatil commented Dec 27, 2025 •

edited

Loading