Skip to content

Refactor Pattern.compile calls to static final fields#207

Open
Copilot wants to merge 10 commits intomainfrom
copilot/refactor-pattern-usage
Open

Refactor Pattern.compile calls to static final fields#207
Copilot wants to merge 10 commits intomainfrom
copilot/refactor-pattern-usage

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 29, 2026

Pattern.compile() was called inside methods and constructors throughout the codebase, recompiling identical regex patterns on every invocation. These should be compiled once as private static final Pattern fields.

Changes

  • Extracted to static fields (previously compiled in methods/constructors):

    • IacrEprintIACR_EPRINT_PATTERN
    • ArXivIdentifierOLD_IDENTIFIER_PATTERN
    • FollowUpQuestionGeneratorNUMBERED_PATTERN
    • StringUtilSPLIT_TEXT_PATTERN
    • CSLFormatUtilsBIBLIOGRAPHY_NUMBER_PATTERN
    • CSLReferenceMarkManagerCITATION_NUMBER_PATTERN
    • CSLCitationOOAdapterCITATION_NUMBER_PATTERN
    • ExtractReferencesActionCOMMENT_NUMBER_PATTERN
    • PreviewTabViewModelXML_TAG_PATTERN, XML_ATTRIBUTES_PATTERN
    • MarkdownDefinitionProviderCITATION_COMMAND_PATTERN, CITATION_KEY_INSIDE_PATTERN
    • ImporterTestWHITESPACE_PATTERN
  • Promoted instance fields to static (were private final, now private static final):

    • CoinsParser — 9 pattern fields (DOI, TITLE, JOURNAL, etc.)
    • MSBibEntryADDRESS_PATTERN
  • Extracted constructor-time compilations to static predicates (PagesChecker):

    // Before: compiled on every PagesChecker instantiation
    isValidPageNumber = Pattern.compile(PAGES_EXP_BIBLATEX).asPredicate();
    
    // After: compiled once
    private static final Predicate<String> IS_VALID_PAGE_NUMBER_BIBLATEX =
        Pattern.compile(PAGES_EXP_BIBLATEX).asPredicate();
  • Added Checkstyle rule (PatternCompileToStaticField) in config/checkstyle/checkstyle.xml to detect future violations. The rule flags any Pattern.compile() call where the argument is a self-contained string literal but the declaration is not static. Dynamic patterns (where the argument is a variable, concatenation with a variable, or a method call like .formatted()) are intentionally excluded.

Dynamic patterns that are built from method parameters (e.g., RegexKeywordGroup, BibTeXConverter, RegExpBasedFileFinder) are intentionally left unchanged, as they cannot be made static.


⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.

Copilot AI changed the title [WIP] Refactor Pattern.compile usage to static field Refactor Pattern.compile calls to static final fields Mar 29, 2026
Copilot AI requested a review from InAnYan March 29, 2026 19:53
Copy link
Copy Markdown
Owner

@InAnYan InAnYan left a comment

Choose a reason for hiding this comment

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

Okay nice. Two things:

  1. Can you check for other patterns in the code and double check that you moved all of them?
  2. Is it possible to make an open rewrite rule or check style rule to keep track when a pattern compiled in a non static code but could be compiled in static? Or something to analyze the code and signal an error?
  3. There might be cases where a pattern is constructed dynamically using some value of a variable - which is NOT a mistake. This shouldn’t be triggered as error, idk how to write this in open rewrite. Maybe open rewrite rule will target only of the argument of pattern compile is a string

Copy link
Copy Markdown
Author

Copilot AI commented Mar 30, 2026

Okay nice. Two things:

  1. Can you check for other patterns in the code and double check that you moved all of them?...

Done in commit c19bdbb. Here's a summary of the three points:

  1. Scan for remaining violations — Scanned the full codebase. One true remaining violation was found: ImporterTest.java:57 used Pattern.compile("\\s") inside a parameterized test method. Fixed by extracting it to private static final WHITESPACE_PATTERN. The remaining non-static Pattern.compile calls are all legitimately dynamic (argument uses a variable or string concatenation with a variable), so they can't be made static.

  2. Checkstyle rule added — Added PatternCompileToStaticField to config/checkstyle/checkstyle.xml. It flags any line containing Pattern.compile( where the first argument is a self-contained string literal (not a variable).

  3. Dynamic patterns are excluded — The rule uses the regex Pattern\.compile\s*\(\s*"(?:[^"\\]|\\.)*"\s*[,)] which only matches when the argument is a complete string literal (closed ") immediately followed by , or ). This means Pattern.compile("prefix" + variable), Pattern.compile(variable), and Pattern.compile("str".formatted(x)) are all safely excluded. Verified against the full codebase — zero false positives.

Copilot AI requested a review from InAnYan March 30, 2026 07:12
@InAnYan InAnYan marked this pull request as ready for review March 30, 2026 19:40
@InAnYan
Copy link
Copy Markdown
Owner

InAnYan commented Mar 30, 2026

TODO: Run the check without refactor and see what would be shown

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.

2 participants