You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We were recently affected by a bug where had stale times/dates for struct defaults --- which is not surprising, just easy to overlook. We ended up writing the cop below which could be generally useful.
I'd be happy to work at creating a PR, but I thought it would be great to get general feedback first. My concerns:
this cop currently isn't smart enough to know whether the prop or const declaration is within a T::Struct definition, so it is likely prone to false positives; it works in our code base fine but I imagine it might need to be a bit more contextual for general usage
this is an easy cop for autocorrect, but I'm still learning rubocop 😅
classStructDefaultTime < CopMSG="Struct property defaults are evaluated only once. Use a factory for time and date defaults"DATE_OR_TIME_METHODS=%i[sincefrom_nowafteragountilbeforeyesterdaytomorrownowtoday].to_set.freezeSTRUCT_DECLARATIONS=%i[constprop].to_set.freezedefon_send(node)struct_default(node)do |_,default_node|
nested_date_or_time(default_node)doadd_offense(node)endendendprivatedef_node_matcher:struct_default,<<~PATTERN (send nil? $STRUCT_DECLARATIONS (sym _) (const nil? _) (hash (pair (sym :default) $(...)))) PATTERNdef_node_matcher:date_or_time,<<~PATTERN (send _ $DATE_OR_TIME_METHODS) PATTERNdefnested_date_or_time(node, &callback)returnifnode.nil? || node.block_type?node.each_child_nodedo |child|
nested_date_or_time(child, &callback)enddate_or_time(node, &callback)endend
The rough test suite looks something like:
classStructDefaultTimeTest < ActiveSupport::TestCaseincludeRubocopTestHelperdefsetup@cop=RuboCop::Cop::Bourgeois::StructDefaultTime.newendtest"ok for non date/time props and constants with or without defaults"doassert_no_offenses("const :foo, Integer, default: 3")assert_no_offenses("prop :bar, String, default: 'foo'")assert_no_offenses("const :baz, Time")endtest"ok for factory date and time props and constants"doassert_no_offenses("const :baz, Time, factory: -> { Time.zone.now }")endtest"offense when using current or relative date/time methods"doassert_offense("const :foo, Date, default: Time.zone.today")assert_offense("const :foo, Time, default: Time.zone.now")assert_offense("const :foo, Date, default: 3.days.from_now")assert_offense("const :foo, Date, default: Time.zone.now.yesterday")assert_offense("const :foo, Time, default: 10.minutes.ago")assert_offense("const :foo, Date, default: 5.days.before(Date.today)")assert_offense("prop :foo, Date, default: Time.zone.today")assert_offense("prop :foo, Time, default: Time.zone.now")assert_offense("prop :foo, Date, default: 3.days.from_now")assert_offense("prop :foo, Date, default: Time.zone.now.yesterday")assert_offense("prop :foo, Time, default: 10.minutes.ago")assert_offense("prop :foo, Date, default: 5.days.before(Date.today)")endend
The text was updated successfully, but these errors were encountered:
We were recently affected by a bug where had stale times/dates for struct defaults --- which is not surprising, just easy to overlook. We ended up writing the cop below which could be generally useful.
I'd be happy to work at creating a PR, but I thought it would be great to get general feedback first. My concerns:
prop
orconst
declaration is within aT::Struct
definition, so it is likely prone to false positives; it works in our code base fine but I imagine it might need to be a bit more contextual for general usageThe rough test suite looks something like:
The text was updated successfully, but these errors were encountered: