Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: error if configuration file has unknown properties #1249

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Sep 15, 2024

Currently the scanner does not check if there were unrecognized properties when loading configuration files, meaning that typos can easily slip through; to help avoid this, I've modified tryLoadConfig to return an error if there are any undecoded keys in the metadata.

While this could cause existing configs to start erroring, I don't think this should be considered a breaking change as such configurations would inherently causing different behaviour than what was desired and thus be a downstream bug themselves.

This will also mean that old versions of the scanner going forward would not be compatible with configs for even newer versions of the scanner that introduce new config properties, which is could be a little annoying but I don't think outweighs the benefit of this validation especially given such configurations likewise would result in different behaviour since the old scanner version would not know what to do with the new configuration option.

Resolves #1098

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.98%. Comparing base (c3295de) to head (ed9fc4a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1249      +/-   ##
==========================================
+ Coverage   67.94%   67.98%   +0.03%     
==========================================
  Files         174      174              
  Lines       16839    16847       +8     
==========================================
+ Hits        11442    11454      +12     
+ Misses       4764     4761       -3     
+ Partials      633      632       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@G-Rath G-Rath marked this pull request as ready for review September 18, 2024 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No warnings when ignoreUntil and effectiveUntil have typos
3 participants