Detect statically identifiable repeat fieldnames and fix offending field in 'endpoint show' #1230
+70
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the RecordPrinter only, emit a warning when duplicate field names are used. This causes tests for `endpoint show` to fail because `Department` is duplicated.
We can detect repeated fieldnames statically when field lists are fully static. Some minimal assumptions are necessary, primarily that a field list is a list of calls to `Field()` and that `Field()` means what we think it does and is always imported under a FromImport. This fails on the same case which is caught by runtime testing, namely endpoint show.
Now that this is implemented in flake8, and there are no dynamic cases which were found with this test which the flake8 lint misses, we do not need to pay a runtime penalty for this enforcement. Remove the check.
Remove the copy of the field which appears higher up. It now only appears adjacent to 'Organization', a semantically similar field.
sirosen
requested review from
MaxTueckeGlobus,
aaschaer,
ada-globus,
derek-globus,
kurtmckee and
m1yag1
as code owners
kurtmckee
approved these changes
Dec 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed that the "Department" field was repeated and explored two techniques for detecting this automatically.
First, runtime detection in printers -- this works but pays a cost at runtime every time a command runs for something we should catch during development.
Testing with this approach verified that it works and that there are no other duplicates in our record-printed fields (at least, under tests).
Second, detection using our local flake8 plugin, adding a rule against duplicate fieldnames in static lists of field objects.
This detects the only real offending example just as well as the runtime check, and only pays the cost at development time (and it is a small cost at that).
Finally, exploration done, remove the runtime check and actually fix the bug by removing the first of the two duplicates.