Add broken contract guidance by abeaufays · Pull Request #371 · seddonym/import-linter

abeaufays · 2026-08-07T15:26:15Z

Summary

Adds optional broken_contract_guidance to the five built-in contract types.
- Guidance appears after a broken contract's violations and can direct the reader to the intended alternative.
Adds reusable TextField support for multi-line configuration text.

Broken contract guidance

broken_contract_guidance lets whoever defines a contract explain how to resolve a failure, so the person whose change broke the contract can see the intended alternative without reading the contract definition.

When a contract with guidance is broken, it is rendered after the violations:

testpackage.high.blue is not allowed to import testpackage.high.green:

- testpackage.high.blue.one -> testpackage.utils (l.3)
  testpackage.utils -> testpackage.high.green (l.3)

Use the testpackage.high top-level interface.
More information here: http://docs.example.com/high

One use case is migrating between systems: a contract can forbid the old system while signposting the new one, instead of leaving the reader to add another entry to the ignore_imports burndown list.

The field is declared on each built-in contract type, so custom contract types do not receive it for free yet. Making it framework-level would require _get_field_names() to walk the MRO; it currently reads cls.__dict__.

Custom contracts can still use the contract_utils function to render it.

TextField

This PR also adds TextField, a reusable field for one text value that may span several lines.

It accepts a normal string, including TOML multiline strings.
It accepts the list of lines produced by the INI reader.
It joins INI lines with newlines without discarding blank lines.

That gives contract code a single string while retaining intentional paragraph breaks in INI configuration. broken_contract_guidance uses TextField; formatting, including bullets and wrapping, is the contract author's choice.

Note: .ini and .toml don't have the exact same behaviour, we can't start a line with a space in an ini file, whereas we can with toml.

Checklist

[x] Add tests for the change.
[x] Add documentation.
[x] Add release notes.

Related to #370
Disclaimer: made with Claude code

abeaufays · 2026-08-07T18:57:37Z

+    if not instructions:
+        return
+
+    output.print_warning("How to fix this:")


Should this be warning-yellow, or a new colour?

Yellow is already the Warnings section's colour, so this may briefly read as "this contract emitted a warning" when it didn't. Alternatives: leave it unstyled (it'd be the only non-red text in the block, so it stands out anyway), or add a dedicated style for guidance. Happy to change it.

Let's just print it as-is for now (i.e. output.print). I think it works better with the way the contracts currently style their breakage messages.

abeaufays · 2026-08-07T18:57:38Z

    allow_indirect_imports = fields.BooleanField(required=False, default=False)
    unmatched_ignore_imports_alerting = fields.EnumField(AlertLevel, default=AlertLevel.ERROR)
    as_packages = fields.BooleanField(required=False, default=True)
+    instructions = fields.ListField(subfield=fields.StringField(), required=False)


instructions is the name from the issue and I'm not attached to it — how_to_fix is more direct about who it's for.

Note the config key and the rendered heading needn't match: the key serves whoever writes the contract, the heading serves whoever broke it. Hence instructions + "How to fix this:".

I think I prefer the term 'guidance' instead of instructions. It would also be nice to be more specific about the context in which this will be shown.

What do you think about calling it breakage_guidance or broken_contract_guidance?

I think I prefer broken_contract_guidance I'm going with that in the next iteration!

abeaufays · 2026-08-07T18:57:38Z


            output.new_line()

+        contract_utils.render_instructions(self.instructions)  # type: ignore


Called at the same position in all five contracts deliberately, so the follow-up that moves this into rendering.py produces identical output.

seddonym

Sorry for the delay in reviewing. I like it!

Have left some small comments, let me know if you have any questions or thoughts.

Thanks @abeaufays !

seddonym · 2026-08-20T14:44:15Z

+    if not instructions:
+        return
+
+    output.print_warning("How to fix this:")


Let's just print it as-is for now (i.e. output.print). I think it works better with the way the contracts currently style their breakage messages.

seddonym · 2026-08-20T14:46:12Z

+    if not instructions:
+        return
+
+    output.print_warning("How to fix this:")


Let's omit this line. I think we should see the printed text as supplementary guidance that is added to the end of the built-in guidance. If someone wants to put 'How to fix this:' in their guidance then they can.

seddonym · 2026-08-20T14:46:54Z

    allow_indirect_imports = fields.BooleanField(required=False, default=False)
    unmatched_ignore_imports_alerting = fields.EnumField(AlertLevel, default=AlertLevel.ERROR)
    as_packages = fields.BooleanField(required=False, default=True)
+    instructions = fields.ListField(subfield=fields.StringField(), required=False)


I think I prefer the term 'guidance' instead of instructions. It would also be nice to be more specific about the context in which this will be shown.

What do you think about calling it breakage_guidance or broken_contract_guidance?

seddonym · 2026-08-20T14:49:50Z

@@ -1,5 +1,9 @@
 # Release notes

+## 2.14 (unreleased)


Can this line be changed to ## latest instead?

(Don't want to commit to that version necessarily.)

seddonym · 2026-08-20T14:53:10Z

    skip_descendants: list[str] | None = None,
    depth: str = "",
    ignore_imports: list[str] | None = None,
+    instructions: list[str] | None = None,


Could we make this into a string by this point? The fact that it happens to be passed as a list is just a consequence of .ini right?

I couldn't find a good way to do this so I'm implementing my TextField suggestion from the PR description for the next iteration.

seddonym · 2026-08-20T14:54:22Z

+        assert capture.get() == dedent(
+            """Illegal imports of protected package mypackage.foo.protected:
+
+- mypackage.foo.sibling -> mypackage.foo.protected.models (l.6)


This could be indented for readability, like the other tests.

I've done this in a last commit.
(I did write this test initially, oopsie)

TextField accepts a string or joins an INI reader's list of lines without discarding blank lines. Assistant-model: Claude Code

Allow built-in contracts to include optional guidance for resolving failures. Guidance is rendered after the contract's violations, so the person who introduced an import can see the intended alternative without reading the contract definition. The option uses TextField, accepting a multi-line value in INI or a TOML multi-line string while preserving blank lines. Assistant-model: Claude Code

These tests need to compare the result of the broken contract rendering against a static string. The static strings were not indented in this specific file.

abeaufays · 2026-08-25T21:32:19Z

@seddonym I've made the change you asked!
But also added the TextField (that you didn't really asked 😅) but I believe that was the best way to address the typing of the broken_contract_guidance field as a string.

This also fixes the INI/TOML selector.

seddonym

Looks great! Thanks.

I took the liberty of pushing a couple of tweaks to the docs, to make it more concise and to fix the INI/TOML switcher.

seddonym · 2026-08-27T16:55:47Z

+
+    def parse(self, raw_data: str | list) -> str:
+        if isinstance(raw_data, list):
+            return "\n".join(raw_data)


I wonder if it's worth calling textwrap.dedent on this so people can indent the text in the contracts, what do you think? (I don't think we need to document that, just a minor improvement.)

I thought the indent was lost even before that step during .ini parsing 🤔
I'm trying on local and can't get it to work 😞

abeaufays · 2026-08-28T10:03:47Z

-    ```
+- `broken_contract_guidance`: A message for Import Linter to display when the contract is broken, below the built-in message. (Optional.) 

+=== "INI"


Oh I did not find this, oops!

abeaufays commented Aug 7, 2026

View reviewed changes

seddonym requested changes Aug 20, 2026

View reviewed changes

abeaufays added 3 commits August 25, 2026 22:51

Add TextField for multi-line text values

af33651

TextField accepts a string or joins an INI reader's list of lines without discarding blank lines. Assistant-model: Claude Code

Indent render broken contract tests in test_protected like others

7d04198

These tests need to compare the result of the broken contract rendering against a static string. The static strings were not indented in this specific file.

abeaufays force-pushed the add-instructions-on-contract branch from 3b2bada to 7d04198 Compare August 25, 2026 21:15

abeaufays changed the title ~~Add instructions option to contracts~~ Add broken contract guidance Aug 25, 2026

abeaufays marked this pull request as ready for review August 25, 2026 21:31

abeaufays requested a review from seddonym August 25, 2026 21:32

Tweak docs for broken_contract_guidance

e6e8233

This also fixes the INI/TOML selector.

seddonym approved these changes Aug 27, 2026

View reviewed changes

seddonym merged commit d314ebb into seddonym:main Aug 27, 2026
6 checks passed

abeaufays commented Aug 28, 2026

View reviewed changes


		output.new_line()

		contract_utils.render_instructions(self.instructions) # type: ignore

Conversation

abeaufays commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Broken contract guidance

TextField

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seddonym left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abeaufays commented Aug 25, 2026

Uh oh!

seddonym left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abeaufays commented Aug 7, 2026 •

edited

Loading