Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* Copyright 2023 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.staticanalysis;

import org.openrewrite.*;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.UsesType;
import org.openrewrite.java.tree.*;

import java.util.Collections;
import java.util.Iterator;
import java.util.Set;

public class ReplaceWeekYearWithYear extends Recipe {
public static final MethodMatcher SIMPLE_DATE_FORMAT_CONSTRUCTOR_MATCHER = new MethodMatcher("java.text.SimpleDateFormat <constructor>(..)");
public static final MethodMatcher OF_PATTERN_MATCHER = new MethodMatcher("java.time.format.DateTimeFormatter ofPattern(..)");
Copy link
Contributor

Choose a reason for hiding this comment

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

Detail: I suggest declaring these constants as private. API surface area is always easy to extend, but more difficult to shrink.


@Override
public String getDisplayName() {
return "Week Year (YYYY) should not be used for date formatting";
}

@Override
public String getDescription() {
return "For most dates Week Year (YYYY) and Year (yyyy) yield the same results. However, on the last week of" +
" December and first week of January Week Year could produce unexpected results.";
}

@Override
public Set<String> getTags() {
return Collections.singleton("RSPEC-3986");
}

@Override
public TreeVisitor<?, ExecutionContext> getVisitor() {
return Preconditions.check(
Preconditions.or(
new UsesType<>("java.util.Date", false),
new UsesType<>("java.time.format.DateTimeFormatter", false),
new UsesType<>("java.text.SimpleDateFormat", false)
),
new ReplaceWeekYearVisitor()
);
}

private static class ReplaceWeekYearVisitor extends JavaIsoVisitor<ExecutionContext> {
@Override
public J.MethodInvocation visitMethodInvocation(J.MethodInvocation mi, ExecutionContext ctx) {
if (OF_PATTERN_MATCHER.matches(mi)) {
getCursor().putMessage("KEY", mi);
}

return super.visitMethodInvocation(mi, ctx);
}

@Override
public J.NewClass visitNewClass(J.NewClass nc, ExecutionContext ctx) {
if (SIMPLE_DATE_FORMAT_CONSTRUCTOR_MATCHER.matches(nc)) {
getCursor().putMessage("KEY", nc);
}

return super.visitNewClass(nc, ctx);
}

@Override
public J.Literal visitLiteral(J.Literal li, ExecutionContext ctx) {
if (li.getValue() instanceof String) {
Cursor c = getCursor().dropParentWhile(is -> is instanceof J.Parentheses || !(is instanceof Tree));
if (c.getMessage("KEY") != null) {
String value = li.getValueSource();
if (value != null && value.contains("YY")) {
Copy link
Member

Choose a reason for hiding this comment

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

Why the check specifically here for at least YY? I'm reading the specification and it seems to allow for single Y as well.. 🤔

For parsing with the abbreviated year pattern ("y" or "yy"), SimpleDateFormat must interpret the abbreviated year relative to some century. It does this by adjusting dates to be within 80 years before and 20 years after the time the SimpleDateFormat instance is created. For example, using a pattern of "MM/dd/yy" and a SimpleDateFormat instance created on Jan 1, 1997, the string "01/11/12" would be interpreted as Jan 11, 2012 while the string "05/04/64" would be interpreted as May 4, 1964. During parsing, only strings consisting of exactly two digits, as defined by Character.isDigit(char), will be parsed into the default century. Any other numeric string, such as a one digit string, a three or more digit string, or a two digit string that isn't all digits (for example, "-1"), is interpreted literally. So "01/02/3" or "01/02/003" are parsed, using the same pattern, as Jan 2, 3 AD. Likewise, "01/02/-3" is parsed as Jan 2, 4 BC.
Otherwise, calendar system specific forms are applied. For both formatting and parsing, if the number of pattern letters is 4 or more, a calendar specific long form is used. Otherwise, a calendar specific short or abbreviated form is used.

Copy link
Member

Choose a reason for hiding this comment

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

And we might need to guard against the (quite hypothetical) use single quotes

Text can be quoted using single quotes (') to avoid interpretation. "''" represents a single quote. All other characters are not interpreted; they're simply copied into the output string during formatting or matched against the input string during parsing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok the latest commits should address these issues

String newValue = value.replace('Y', 'y');
return li.withValueSource(newValue).withValue(newValue);
}
}
}

return li;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
/*
* Copyright 2023 the original author or authors.
* <p>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openrewrite.staticanalysis;

import org.junit.jupiter.api.Test;
import org.junitpioneer.jupiter.Issue;
import org.openrewrite.DocumentExample;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.java.Assertions.java;

@SuppressWarnings("SuspiciousDateFormat")
class ReplaceWeekYearWithYearTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec
.recipe(new ReplaceWeekYearWithYear());
}

@Test
@DocumentExample
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/58")
void changeSimpleDateFormat() {
//language=java
rewriteRun(
java(
"""
import java.text.SimpleDateFormat;
import java.util.Date;

class Test {
public void formatDate() {
Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31");
String result = new SimpleDateFormat("YYYY/MM/dd").format(date);
}
}
""",
"""
import java.text.SimpleDateFormat;
import java.util.Date;

class Test {
public void formatDate() {
Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31");
String result = new SimpleDateFormat("yyyy/MM/dd").format(date);
}
}
"""
)
);
}

@Test
void worksWithOfPatternFormatter() {
//language=java
rewriteRun(
java(
"""
import java.text.SimpleDateFormat;
import java.time.format.DateTimeFormatter;
import java.util.Date;

class Test {
public void formatDate() {
Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31");
String result = DateTimeFormatter.ofPattern("YYYY/MM/dd").format(date.toInstant());
}
}
""",
"""
import java.text.SimpleDateFormat;
import java.time.format.DateTimeFormatter;
import java.util.Date;

class Test {
public void formatDate() {
Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31");
String result = DateTimeFormatter.ofPattern("yyyy/MM/dd").format(date.toInstant());
}
}
"""
)
);
}

@Test
void worksWithYYUses() {
//language=java
rewriteRun(
java(
"""
import java.text.SimpleDateFormat;
import java.time.format.DateTimeFormatter;
import java.util.Date;

class Test {
public void formatDate() {
Date date = new SimpleDateFormat("yy/MM/dd").parse("2015/12/31");
String result = DateTimeFormatter.ofPattern("YY/MM/dd").format(date.toInstant());
}
}
""",
"""
import java.text.SimpleDateFormat;
import java.time.format.DateTimeFormatter;
import java.util.Date;

class Test {
public void formatDate() {
Date date = new SimpleDateFormat("yy/MM/dd").parse("2015/12/31");
String result = DateTimeFormatter.ofPattern("yy/MM/dd").format(date.toInstant());
}
}
"""
)
);
}

@Test
void onlyRunsWhenFormatAndDateTypesAreUsed() {
//language=java
rewriteRun(
java(
"""
class Test {
public static void main(String[] args) {
String pattern = "YYYY/MM/dd";
System.out.println(pattern);
}
}
"""
)
);
}

@Test
void standaloneNewClassCall() {
//language=java
rewriteRun(
java(
"""
import java.text.SimpleDateFormat;
import java.util.Date;

class Test {
public void formatDate() {
SimpleDateFormat format = new SimpleDateFormat("YYYY-MM-dd");
Date date = format.parse("2015/12/31");
}
}
""",
"""
import java.text.SimpleDateFormat;
import java.util.Date;

class Test {
public void formatDate() {
SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd");
Date date = format.parse("2015/12/31");
}
}
"""
)
);
}
}