Skip to content

Add has_timestamp linter#682

Merged
morgo merged 4 commits into
block:mainfrom
morgo:lint-has-timestamp
Apr 6, 2026
Merged

Add has_timestamp linter#682
morgo merged 4 commits into
block:mainfrom
morgo:lint-has-timestamp

Conversation

@morgo
Copy link
Copy Markdown
Collaborator

@morgo morgo commented Apr 6, 2026

A Pull Request should be associated with an Issue.

We wish to have discussions in Issues. A single issue may be targeted by multiple PRs.
If you're offering a new feature or fixing anything, we'd like to know beforehand in Issues,
and potentially we'll be able to point development in a particular direction.
Further notes in https://github.com/block/spirit/blob/main/.github/CONTRIBUTING.md

Fixes #668

Copy link
Copy Markdown
Contributor

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

Adds a new has_timestamp linter intended to detect MySQL TIMESTAMP columns (Y2038 risk) and report violations during schema linting and migration linting.

Changes:

  • Introduces HasTimestampLinter that flags TIMESTAMP usage in CREATE TABLE and ALTER TABLE statements with error/warning severities.
  • Adds a comprehensive test suite covering CREATE TABLE, ALTER TABLE, registration, and edge cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
pkg/lint/lint_has_timestamp.go Implements the has_timestamp linter logic for CREATE/ALTER analysis and violation generation.
pkg/lint/lint_has_timestamp_test.go Adds unit tests validating detection, severity, and metadata/registration behaviors.

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

Comment thread pkg/lint/lint_has_timestamp.go
Comment thread pkg/lint/lint_has_timestamp.go
Comment thread pkg/lint/lint_has_timestamp_test.go
Comment thread pkg/lint/lint_has_timestamp_test.go Outdated
morgo added 2 commits April 6, 2026 10:42
- Fix severity for existing tables: existingTables now produce Warning
  (not Error) to avoid blocking all changes to legacy schemas with
  TIMESTAMP columns ('don't boil the ocean')
- Fix severity for CREATE TABLE: only CREATE TABLE in changes (new
  tables) produce Error; existing table schemas produce Warning
- Exclude columns being fixed from ALTER warnings: DROP COLUMN,
  MODIFY to non-TIMESTAMP, and CHANGE to non-TIMESTAMP no longer
  produce false-positive warnings when the ALTER is actively fixing
  the TIMESTAMP problem
- Add new tests for fix scenarios: ALTER DROP TIMESTAMP column,
  ALTER MODIFY TIMESTAMP to DATETIME, ALTER CHANGE TIMESTAMP to
  DATETIME, ALTER DROP all TIMESTAMP columns
- Update existing tests to match corrected severity expectations
@morgo morgo requested a review from aparajon April 6, 2026 22:44
@morgo morgo merged commit f91fdb2 into block:main Apr 6, 2026
12 checks passed
@morgo morgo deleted the lint-has-timestamp branch April 6, 2026 23:50
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.

New linter: has_timestamp — Y2038 TIMESTAMP detection

3 participants