Skip to content

Commit

Permalink
add some warnings reported only in parse mode
Browse files Browse the repository at this point in the history
  • Loading branch information
radeusgd committed Sep 22, 2023
1 parent 2b987a9 commit 9bbaab9
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import project.Data.Time.Errors.Date_Time_Format_Parse_Error
import project.Data.Time.Time_Of_Day.Time_Of_Day
import project.Error.Error
import project.Errors.Illegal_Argument.Illegal_Argument
import project.Errors.Problem_Behavior.Problem_Behavior
import project.Errors.Time_Error.Time_Error
import project.Nothing.Nothing
import project.Panic.Panic
Expand All @@ -23,7 +24,7 @@ polyglot java import org.enso.base.time.FormatterKind

type Date_Time_Formatter
## PRIVATE
Value (underlying : EnsoDateTimeFormatter)
Value (underlying : EnsoDateTimeFormatter) (~deferred_parsing_warnings = [])

## Creates a formatter from a simple date-time format pattern.

Expand Down Expand Up @@ -127,7 +128,7 @@ type Date_Time_Formatter
analyzer = Analyzer.new parsed
java_formatter = analyzer.validate_after_parsing <|
As_Java_Formatter_Interpreter.interpret locale parsed
Date_Time_Formatter.Value (EnsoDateTimeFormatter.new java_formatter pattern FormatterKind.SIMPLE)
Date_Time_Formatter.Value (EnsoDateTimeFormatter.new java_formatter pattern FormatterKind.SIMPLE) deferred_parsing_warnings=analyzer.get_parsing_only_warnings

## Creates a formatter from a pattern for the ISO 8601 leap week calendar.

Expand Down Expand Up @@ -177,7 +178,7 @@ type Date_Time_Formatter
analyzer = Analyzer.new parsed
java_formatter = analyzer.validate_after_parsing <|
As_Java_Formatter_Interpreter.interpret locale parsed
Date_Time_Formatter.Value (EnsoDateTimeFormatter.new java_formatter pattern FormatterKind.ISO_WEEK_DATE)
Date_Time_Formatter.Value (EnsoDateTimeFormatter.new java_formatter pattern FormatterKind.ISO_WEEK_DATE) deferred_parsing_warnings=analyzer.get_parsing_only_warnings

## ADVANCED
Creates a formatter from a Java `DateTimeFormatter` instance or a text
Expand Down Expand Up @@ -284,15 +285,18 @@ type Date_Time_Formatter

## PRIVATE
parse_date self (text:Text) = self.handle_java_errors <|
self.underlying.parseLocalDate text
self.with_parsing_warnings <|
self.underlying.parseLocalDate text

## PRIVATE
parse_date_time self (text:Text) = self.handle_java_errors <|
self.underlying.parseZonedDateTime text
self.with_parsing_warnings <|
self.underlying.parseZonedDateTime text

## PRIVATE
parse_time self (text:Text) = self.handle_java_errors <|
self.underlying.parseLocalTime text
self.with_parsing_warnings <|
self.underlying.parseLocalTime text

## PRIVATE
format_date self (date:Date) = self.handle_java_errors <|
Expand All @@ -306,6 +310,18 @@ type Date_Time_Formatter
format_time self (time:Time_Of_Day) = self.handle_java_errors <|
self.underlying.formatLocalTime time

## PRIVATE
Adds parsing warnings, if any, to the result of `continuation`.
with_parsing_warnings self ~continuation =
Problem_Behavior.Report_Warning.attach_problems_after continuation self.deferred_parsing_warnings

## PRIVATE
Returns the `underlying` formatter, also ensuring that parse-only
warnings are attached to it, to be propagated.
get_java_formatter_for_parsing : EnsoDateTimeFormatter
get_java_formatter_for_parsing self =
self.with_parsing_warnings self.underlying

## PRIVATE
Date_Time_Formatter.from (that:Text) (locale:Locale = Locale.default) =
Date_Time_Formatter.from_simple_pattern that locale
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ type Analyzer
self.check_24h_and_ampm_collision problem_builder
Problem_Behavior.Report_Warning.attach_problems_after continuation problem_builder.to_vector

## PRIVATE
Prepares a list of warnings that are only reported when parsing using the
formatter.
get_parsing_only_warnings : Vector
get_parsing_only_warnings self =
problem_builder = Vector.new_builder
self.check_missing_ampm_in_hour_parse problem_builder
self.check_missing_year_in_date_parse problem_builder
problem_builder.to_vector

## PRIVATE
check_possible_m_mismatches self problem_builder =
pattern_nodes = self.flattened
Expand Down Expand Up @@ -80,7 +90,7 @@ type Analyzer
Time_Patterns.Second _ -> True
_ -> False
if seconds.length == 2 then
problem_builder.append (Suspicious_Date_Time_Format.Warning "Two second patterns have been detected ('s'/'S'). Our simple format treats seconds in a case-insensitive way. If you want to indicate a fraction of a second, use 'f' instead. (You can remove this warning using `remove_warnings Suspicious_Date_Time_Format`.)")
problem_builder.append (Suspicious_Date_Time_Format.Warning "Two Second patterns have been detected ('s'/'S'). Our simple format treats seconds in a case-insensitive way. If you want to indicate a fraction of a second, use 'f' instead. (You can remove this warning using `remove_warnings Suspicious_Date_Time_Format`.)")

## PRIVATE
has_24h : Boolean
Expand All @@ -107,3 +117,21 @@ type Analyzer
check_24h_and_ampm_collision self problem_builder =
if self.has_24h && self.has_ampm && self.has_12h.not then
problem_builder.append (Suspicious_Date_Time_Format.Warning "A 24-hour pattern 'H' is used with an AM/PM pattern. Did you mean 'h' for 12-hour format? (You can remove this warning using `remove_warnings Suspicious_Date_Time_Format`.)")

## PRIVATE
check_missing_ampm_in_hour_parse self problem_builder =
if self.has_12h && self.has_ampm.not then
problem_builder.append (Suspicious_Date_Time_Format.Warning "A 12-hour pattern 'h' is used without an AM/PM pattern. Without it, the 12-hour pattern is ambiguous - the hours will default to AM. Did you mean 'H' for 24-hour format? (You can remove this warning using `remove_warnings Suspicious_Date_Time_Format`.)")

## PRIVATE
has_day_and_month_but_not_year : Boolean
has_day_and_month_but_not_year self =
has_month = self.has_required Standard_Date_Patterns.Month
has_day = self.has_required Standard_Date_Patterns.Day_Of_Month
has_year = self.has_required Standard_Date_Patterns.Year
has_month && has_day && has_year.not

## PRIVATE
check_missing_year_in_date_parse self problem_builder =
if self.has_day_and_month_but_not_year then
problem_builder.append (Suspicious_Date_Time_Format.Warning "A date pattern with a day and month but without a year has been detected. The year will default to the current year - note that the results may change over time. (You can remove this warning using `remove_warnings Suspicious_Date_Time_Format`.)")
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,9 @@ interpret locale nodes prepare_defaults=True =
if analyzer.has_required Standard_Date_Patterns.Day_Of_Month . not then
builder.parseDefaulting ChronoField.DAY_OF_MONTH 1

if analyzer.has_required Standard_Date_Patterns.Month && analyzer.has_required Standard_Date_Patterns.Day_Of_Month then
if analyzer.has_required Standard_Date_Patterns.Year . not then
current_year = Date.today.year
builder.parseDefaulting ChronoField.YEAR current_year
if analyzer.has_day_and_month_but_not_year then
current_year = Date.today.year
builder.parseDefaulting ChronoField.YEAR current_year

if analyzer.has_required ISO_Week_Year_Patterns.Week_Based_Year && analyzer.has_required ISO_Week_Year_Patterns.Week_Of_Year then
if analyzer.has_required ISO_Week_Year_Patterns.Day_Of_Week . not then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,17 +204,17 @@ type Data_Formatter
## PRIVATE
make_date_parser self = self.wrap_base_parser <|
Panic.catch Java_Exception handler=(caught_panic-> Error.throw (Illegal_Argument.Error caught_panic.payload.getMessage)) <|
DateParser.new (self.date_formats.map .underlying)
DateParser.new (self.date_formats.map .get_java_formatter_for_parsing)

## PRIVATE
make_date_time_parser self = self.wrap_base_parser <|
Panic.catch Java_Exception handler=(caught_panic-> Error.throw (Illegal_Argument.Error caught_panic.payload.getMessage)) <|
DateTimeParser.new (self.datetime_formats.map .underlying)
DateTimeParser.new (self.datetime_formats.map .get_java_formatter_for_parsing)

## PRIVATE
make_time_of_day_parser self = self.wrap_base_parser <|
Panic.catch Java_Exception handler=(caught_panic-> Error.throw (Illegal_Argument.Error caught_panic.payload.getMessage)) <|
TimeOfDayParser.new (self.time_formats.map .underlying)
TimeOfDayParser.new (self.time_formats.map .get_java_formatter_for_parsing)

## PRIVATE
make_identity_parser self = self.wrap_base_parser IdentityParser.new
Expand Down
15 changes: 12 additions & 3 deletions test/Tests/src/Data/Time/Date_Time_Formatter_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,8 @@ spec =
r1.should_equal (Time_Of_Day.new 4 24)
w1 = Problems.expect_only_warning Suspicious_Date_Time_Format r1
w1.to_display_text . should_contain "ambiguous"
w1.to_display_text . should_contain "default to AM"
w1.to_display_text . should_contain "Did you mean 'H'"

Test.specify "should allow to parse MM-dd without a year, defaulting to current year but adding a warning (only in parsing)" <|
f1 = Date_Time_Formatter.from "dd.MM"
Expand All @@ -308,10 +310,17 @@ spec =
c2.to_vector . should_equal [Date.new current_year 12 31, Date.new current_year 01 01]
Problems.expect_only_warning Suspicious_Date_Time_Format c2

c3 = Column.from_vector "strs" ["04:24", "16:24"]
c3 = Column.from_vector "strs" ["04:24", "16:25"]
t3 = c3.to_table
t4 = t3.parse type=Value_Type.Time format="hh:mm"
t4.at "strs" . to_vector . should_equal [Time_Of_Day.new 4 24, Time_Of_Day.new 4 24]
Problems.expect_only_warning Suspicious_Date_Time_Format t4
# The entry `16:25` does not fit the 12h format, so it is not parsed.
t4.at "strs" . to_vector . should_equal [Time_Of_Day.new 4 24, Nothing]
Problems.expect_warning Suspicious_Date_Time_Format t4

# But no warnings on format
c5 = Column.from_vector "Y" [Date.new 2023 12 25, Date.new 2011 07 31]
c6 = c5.format "dd.MM"
c6.to_vector . should_equal ["25.12", "31.07"]
Problems.assume_no_problems c6

main = Test_Suite.run_main spec

0 comments on commit 9bbaab9

Please sign in to comment.