Skip to content

Add validation for config file extensions with clear error messages #691

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.

Sign up for GitHub

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

Jump to bottom
Open
Copilot wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add validation for config file extensions with clear error messages #691

Copilot wants to merge 7 commits into from
+178 −2

Conversation

Copy link
Contributor

Copilot AI commented Feb 2, 2026 •
edited
Loading

Plan: Add validation/warning for unsupported config file extensions

  • Understand the current code structure and file validation approach
  • Add file extension validation function in internal/config/viper.go
  • Update SetFileName() in internal/config/config.go to validate extension before calling configureViper()
  • Create appropriate localized error messages using localizer.Errorf()
  • Add comprehensive tests in internal/config/viper_test.go for:
    • Valid extensions (.yaml, .yml)
    • No extension (for default sqlconfig file)
    • Invalid extensions (.txt, .json, etc.)
    • Edge cases (multiple dots, hidden files, files with only dot)
  • Run tests to verify the implementation - all tests pass
  • Manually test the error messages with invalid config files
  • Address code review feedback (whitespace and import ordering)
  • Run security scans - no issues found
  • Address review comment: Validate extension BEFORE creating file
  • Format error message as single line for consistency
  • Add edge case tests for improved coverage
  • Complete implementation
Original prompt

This section details on the original issue you should resolve

<issue_title>Add validation/warning for unsupported config file extensions</issue_title>
<issue_description>Description:
Currently, when users specify a config file with an unsupported extension (e.g., --sqlconfig myfile.txt), they receive a confusing Viper error: "Unsupported Config Type 'txt'".

Proposed Enhancement:
Add validation in internal/config/viper.go to check the file extension before passing to Viper and provide a clearer error message such as:

Error: Configuration files must use YAML format with .yaml or .yml extension.
The file 'myfile.txt' has an unsupported extension '.txt'.

Related:

Implementation Notes:

  • Check file extension in configureViper() or SetFileName() function
  • Allow .yaml, .yml, and no extension (for default ~/.sqlcmd/sqlconfig)
  • Provide helpful error message pointing users to documentation</issue_description>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits February 2, 2026 00:25
@dlevy-msft-sql
Add validation for unsupported config file extensions
3a206e4 
Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
@dlevy-msft-sql
Fix trailing whitespace in viper.go
598dce0 
Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
@dlevy-msft-sql
Organize imports in standard Go convention
eeb651b 
Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
Copilot AI changed the title [WIP] Add validation for unsupported config file extensions Add validation for config file extensions with clear error messages Feb 2, 2026
Copilot AI requested a review from dlevy-msft-sql February 2, 2026 00:29
@dlevy-msft-sql dlevy-msft-sql added documentation Improvements or additions to documentation enhancement New feature or request Priority: 1 High priority/impact Priority: 3 Low priority/impact Size: S Small issue (less than one week effort) needs-work labels Feb 2, 2026
Copilot AI reviewed Feb 2, 2026
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds validation for configuration file extensions to provide clear, user-friendly error messages when users specify unsupported file extensions for the --sqlconfig flag. Previously, users would receive Viper's cryptic "Unsupported Config Type" error.

Changes:

  • Added validateConfigFileExtension() function to check file extensions (.yaml, .yml, or no extension) in a case-insensitive manner
  • Modified configureViper() to return an error instead of void, enabling proper error propagation
  • Added comprehensive test coverage with 10 test cases covering valid extensions, invalid extensions, and edge cases

Reviewed changes

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

File Description
internal/config/viper.go Implements validation logic for config file extensions and updates configureViper to return errors
internal/config/config.go Updates SetFileName to handle errors from configureViper using checkErr
internal/config/viper_test.go Adds comprehensive test coverage for extension validation including valid, invalid, and edge cases

@dlevy-msft-sql
Copy link
Contributor

dlevy-msft-sql commented Feb 2, 2026

@copilot you've got review comments to fix

@dlevy-msft-sql
Validate file extension before creating empty file
6e43792 
Move validateConfigFileExtension call before file.CreateEmptyIfNotExists
to avoid creating unnecessary files with invalid extensions on the filesystem.
This addresses the review comment by ensuring validation happens first.

Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
@dlevy-msft-sql dlevy-msft-sql removed the Priority: 1 High priority/impact label Feb 2, 2026
Copilot AI reviewed Feb 2, 2026
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@dlevy-msft-sql
Copy link
Contributor

dlevy-msft-sql commented Feb 2, 2026

@copilot fix the review comment

@dlevy-msft-sql
Format error message as single line for consistency
9ae247f 
Changed multi-line error message to single-line format to maintain
consistency with error message patterns throughout the codebase.

Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
Copilot AI reviewed Feb 2, 2026
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@dlevy-msft-sql
Copy link
Contributor

dlevy-msft-sql commented Feb 2, 2026

@copilot fix the review issues

@dlevy-msft-sql
Add edge case tests for file extension validation
9a00d06 
Added test cases for edge scenarios:
- Files with multiple dots (my.config.yaml, config.backup.txt)
- Hidden files with extensions (.config.yaml, .config.txt)
- Files with only dot and extension (.yaml, .yml)

Total test cases increased from 10 to 19, improving coverage for
various filename patterns that users might encounter.

Co-authored-by: dlevy-msft-sql <194277063+dlevy-msft-sql@users.noreply.github.com>
Copilot AI reviewed Feb 2, 2026
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@dlevy-msft-sql dlevy-msft-sql marked this pull request as ready for review February 2, 2026 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@dlevy-msft-sql dlevy-msft-sql Awaiting requested review from dlevy-msft-sql

@shueybubbles shueybubbles Awaiting requested review from shueybubbles

At least 1 approving review is required to merge this pull request.

Assignees

@dlevy-msft-sql dlevy-msft-sql

Copilot code review Copilot

Labels

documentation Improvements or additions to documentation enhancement New feature or request in-review Priority: 3 Low priority/impact Size: S Small issue (less than one week effort)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add validation/warning for unsupported config file extensions

2 participants

@dlevy-msft-sql