Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix modernizer #9824

Merged
merged 4 commits into from
May 4, 2023
Merged

Fix modernizer #9824

merged 4 commits into from
May 4, 2023

Conversation

koppor
Copy link
Member

@koppor koppor commented May 1, 2023

Modernizer starts to complain a lot in our source code.

Many times:

TreeCollector.java:64: Prefer java.util.Optional.orElseThrow

I disabled that check.

The other checks were fine. All fixed. This is a hotfix and blocks merge of #9811.

C:\git-repositories\jabref-all\jabref\src\main\java\org\jabref\gui\push\PushToLyx.java:89: Prefer java.io.FileWriter.<init>(File, Charset)
C:\git-repositories\jabref-all\jabref\src\main\java\org\jabref\logic\importer\fetcher\DoiFetcher.java:95: Prefer java.lang.Thread.threadId
C:\git-repositories\jabref-all\jabref\src\main\java\org\jabref\logic\l10n\Language.java:55: Prefer java.util.Locale.of
C:\git-repositories\jabref-all\jabref\src\main\java\org\jabref\logic\l10n\Language.java:57: Prefer java.util.Locale.of
C:\git-repositories\jabref-all\jabref\src\test\java\org\jabref\logic\importer\fetcher\CompositeSearchBasedFetcherTest.java:53: Prefer java.util.Collections.emptyList()
C:\git-repositories\jabref-all\jabref\src\test\java\org\jabref\logic\importer\fetcher\CompositeSearchBasedFetcherTest.java:63: Prefer java.util.Collections.emptyList()
C:\git-repositories\jabref-all\jabref\src\test\java\org\jabref\logic\integrity\UTF8CheckerTest.java:45: Prefer java.lang.String.<init>(byte[], java.nio.charset.Charset)
C:\git-repositories\jabref-all\jabref\src\test\java\org\jabref\logic\integrity\UTF8CheckerTest.java:59: Prefer java.lang.String.<init>(byte[], java.nio.charset.Charset)
C:\git-repositories\jabref-all\jabref\src\test\java\org\jabref\logic\integrity\UTF8CheckerTest.java:60: Prefer java.lang.String.getBytes(java.nio.charset.Charset)
C:\git-repositories\jabref-all\jabref\src\test\java\org\jabref\logic\l10n\LanguageTest.java:15: Prefer java.util.Locale.of
C:\git-repositories\jabref-all\jabref\src\test\java\org\jabref\logic\l10n\LanguageTest.java:21: Prefer java.util.Locale.of

Compulsory checks
Preview Give feedback

@koppor koppor added the dev: code-quality Issues related to code or architecture decisions label May 1, 2023
Siedlerchr
Siedlerchr reviewed May 1, 2023
View reviewed changes
src/test/java/org/jabref/logic/integrity/UTF8CheckerTest.java
@@ -42,7 +44,7 @@ void fieldAcceptsUTF8() {
@Test
void fieldDoesNotAcceptUmlauts() throws UnsupportedEncodingException {
UTF8Checker checker = new UTF8Checker(Charset.forName("GBK"));
String NonUTF8 = new String("你好，这条语句使用GBK字符集".getBytes(), "GBK");
String NonUTF8 = new String("你好，这条语句使用GBK字符集".getBytes(StandardCharsets.UTF_8), GBK);
Copy link
Member

Choose a reason for hiding this comment

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

Sure that this is intended here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Test goes through. JavaDoc on getBytes says that default Charset is used. Maybe be platform dependend. Java switched to UTF8 recently.

See the last parameter GBK which recodes the bytes to GKB leading to non-utf. Therefore, I think, it works.

Siedlerchr
Siedlerchr reviewed May 1, 2023
View reviewed changes
koppor
koppor commented May 2, 2023
View reviewed changes
@koppor
Copy link
Member Author

koppor commented May 2, 2023

If modernizer does not fail, does any one look at it?

Should we factor it out to a separate gradle task - similar to checkstyle?

@Siedlerchr
Copy link
Member

I would say we leave it as is. Most often the IDE already tells you that you can improve it

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 2, 2023
@koppor
Copy link
Member Author

koppor commented May 2, 2023

I would say we leave it as is. Most often the IDE already tells you that you can improve it

Similar to checkstyle 🤣🤣🤣

@calixtus calixtus merged commit 0d30f7d into JabRef:main May 4, 2023
@calixtus calixtus deleted the fix-modernizer branch May 4, 2023 17:43
Siedlerchr added a commit that referenced this pull request May 4, 2023
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into addPrefsForBak
f6688b8 
* upstream/main:
  Fix modernizer (#9824)
  Improve search history by attaching change listener (#9794)
  Fix split multiline localization (#9814)
  New Crowdin updates (#9834)
  change versin to 0.8.10
  remove  jacoco version config
Siedlerchr added a commit to brunaoo/jabref that referenced this pull request May 5, 2023
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-for-issue-9737
50b18f7 
* upstream/main: (88 commits)
  Minimal config for openRewrite
  Fix missing #
  Fix modernizer (JabRef#9824)
  CHANGELOG.md
  Removed unused code
  Refined ui
  Dissolved FileTab and moved contents to EntryTab
  Renamed CustomEditorFieldsTab to EntryEditorTabsTab
  Fixed antipattern, fixed radiobutton with checkbox
  Renamed ImportExportPreferences to ExportPreferences
  Improve search history by attaching change listener (JabRef#9794)
  Separated WebSearchPrefs and ExportPrefs
  Separated WebSearchTab and ExportTab
  Renamed ImportExportTab to WebSearchTab
  Fix split multiline localization (JabRef#9814)
  New Crowdin updates (JabRef#9834)
  change versin to 0.8.10
  remove  jacoco version config
  Bump org.junit.platform:junit-platform-launcher from 1.9.2 to 1.9.3
  Bump org.jsoup:jsoup from 1.15.4 to 1.16.1
  ...
Siedlerchr added a commit to GuyPuts/jabref that referenced this pull request May 6, 2023
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into issue-2573-support-…
6fd2603 
…biblatex-date-formats

* upstream/main: (132 commits)
  Add four rules not having any effect
  Apply BooleanChecksNotInverted
  Remove unused RadioButtonCell
  Minimal config for openRewrite
  Fix missing #
  Fix modernizer (JabRef#9824)
  CHANGELOG.md
  Removed unused code
  Refined ui
  Dissolved FileTab and moved contents to EntryTab
  Renamed CustomEditorFieldsTab to EntryEditorTabsTab
  Fixed antipattern, fixed radiobutton with checkbox
  Renamed ImportExportPreferences to ExportPreferences
  Improve search history by attaching change listener (JabRef#9794)
  Separated WebSearchPrefs and ExportPrefs
  Separated WebSearchTab and ExportTab
  Renamed ImportExportTab to WebSearchTab
  Fix split multiline localization (JabRef#9814)
  New Crowdin updates (JabRef#9834)
  change versin to 0.8.10
  ...
Siedlerchr added a commit to yenniejunvu/jabref that referenced this pull request May 7, 2023
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into fix-for-9682
90bd191 
* upstream/main: (110 commits)
  remove extra hack rather add a todo
  Enable CompareEnumsWithEqualityOperator
  cleanup
  checkstyle
  checkstyle
  Prefer u over y, remove some patterns that have not a  test yet make parameterized test
  Enable ChainStringBuilderAppendCalls
  Add four rules not having any effect
  Apply BooleanChecksNotInverted
  Remove unused RadioButtonCell
  move exception handling upwards
  Minimal config for openRewrite
  Fix missing #
  Update CHANGELOG.md
  exchange locations between library mode and library encoding added to Changelog
  exchange locations between library mode and library encoding in general tab
  Fix modernizer (JabRef#9824)
  CHANGELOG.md
  Removed unused code
  Refined ui
  ...

# Conflicts:
#	src/main/java/org/jabref/logic/exporter/ModsExporter.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees
No one assigned
Labels
dev: code-quality Issues related to code or architecture decisions status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@koppor @Siedlerchr @calixtus