From c3a7c0a0c6a6c701ed552d028546dd54b88b2af8 Mon Sep 17 00:00:00 2001 From: Christoph Pirkl <4711730+kaklakariada@users.noreply.github.com> Date: Sun, 14 Apr 2024 17:12:49 +0200 Subject: [PATCH] Fix sonar warnings (#274) * Upgrade dependencies * Fix startup failure under Linux java.lang.UnsupportedOperationException: Unable to load glass GTK library. at com.sun.glass.ui.gtk.GtkApplication.lambda$new$5(GtkApplication.java:176) at java.base/java.security.AccessController.doPrivileged(AccessController.java:319) at com.sun.glass.ui.gtk.GtkApplication.(GtkApplication.java:163) * Add changelog entry * Add prev/next buttons to monthly report * Fix indentation * Fix uitest * Add changelog entry * Disable null analysis * Improve test stabilitiy * Fix build error in vscode * Reduce log verbosity * Poll for condition * Avoid warning during uitests * Improve logging * Disable uitest * Archive test reports * Mark test as flaky * Archive test results * Fix sonar findings * Fix archiving reports * Revert changes * Fix sonar finding * Disable null analysis for eclipse * Update codeql build --------- Co-authored-by: kaklakariada --- .github/workflows/build.yml | 23 +++++++++---- .github/workflows/codeql-analysis.yml | 33 +++++++++++-------- jfxui/.settings/org.eclipse.jdt.core.prefs | 10 +++--- .../jfxui/ui/DailyProjectReportViewer.java | 18 +++++----- .../jfxui/MonthlyProjectReportTest.java | 5 +-- .../logic/model/DayActivities.java | 14 ++++---- .../whiterabbit/logic/model/DayRecord.java | 26 +++++++-------- .../project/ProjectReportGenerator.java | 12 +++---- .../singleinstance/ClientConnection.java | 14 +++----- .../CalculatedHolidaysTest.java | 2 +- 10 files changed, 83 insertions(+), 74 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b3525331..b2fac7d8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -52,6 +52,22 @@ jobs: run: | ./gradlew build --info --warning-mode=summary -PskipFlakyTests=true -PjavaVersion=${{ matrix.java }} + - name: Archive executable JAR + uses: actions/upload-artifact@v4 + if: ${{ env.DEFAULT_JAVA == matrix.java && env.DEFAULT_OS == matrix.os }} + with: + name: executable-jar + path: jfxui/build/libs/white-rabbit-fx-*.jar + if-no-files-found: error + + - name: Archive test reports for ${{ matrix.os }} using Java ${{ matrix.java }} + uses: actions/upload-artifact@v4 + if: ${{ always() }} + with: + name: test-reports-${{ matrix.os }}-java-${{ matrix.java }} + path: "**/build/reports/tests/*/**" + if-no-files-found: error + - name: Sonar analysis if: ${{ env.DEFAULT_JAVA == matrix.java && env.DEFAULT_OS == matrix.os && env.SONAR_TOKEN != null }} run: | @@ -60,13 +76,6 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - - name: Archive executable JAR for ${{ matrix.java }} - uses: actions/upload-artifact@v4 - if: ${{ env.DEFAULT_JAVA == matrix.java && env.DEFAULT_OS == matrix.os }} - with: - name: packages - path: jfxui/build/libs/white-rabbit-fx-*.jar - - name: Build native package for ${{ runner.os }} using Java ${{ matrix.java }} if: ${{ env.DEFAULT_JAVA == matrix.java }} run: | diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 84117ddd..8e84d81c 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -2,34 +2,40 @@ name: "CodeQL" on: push: - branches: [ main ] + branches: [ "main" ] pull_request: - branches: [ main ] + branches: [ "main" ] schedule: - - cron: '0 7 * * 6' + - cron: '18 19 * * 5' jobs: analyze: concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true - name: Analyze + name: Analyze (${{ matrix.language }}) runs-on: ubuntu-latest + timeout-minutes: 360 permissions: - contents: read + # required for all workflows security-events: write + # required to fetch internal or private CodeQL packs + packages: read + + # only required for workflows in private repositories + actions: read + contents: read + strategy: fail-fast: false matrix: - language: ['java'] - + include: + - language: java-kotlin + build-mode: none # This mode only analyzes Java. Set this to 'autobuild' or 'manual' to analyze Kotlin too. steps: - name: Checkout repository uses: actions/checkout@v4 - with: - fetch-depth: 2 - - uses: actions/setup-java@v4 with: distribution: 'temurin' @@ -42,9 +48,10 @@ jobs: uses: github/codeql-action/init@v3 with: languages: ${{ matrix.language }} - - - name: Autobuild - uses: github/codeql-action/autobuild@v3 + build-mode: ${{ matrix.build-mode }} + queries: security-extended,security-and-quality - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v3 + with: + category: "/language:${{matrix.language}}" diff --git a/jfxui/.settings/org.eclipse.jdt.core.prefs b/jfxui/.settings/org.eclipse.jdt.core.prefs index c507a4ce..d97af520 100644 --- a/jfxui/.settings/org.eclipse.jdt.core.prefs +++ b/jfxui/.settings/org.eclipse.jdt.core.prefs @@ -37,7 +37,7 @@ org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=org.eclipse.jdt.annota org.eclipse.jdt.core.compiler.annotation.nonnullbydefault.secondary= org.eclipse.jdt.core.compiler.annotation.nullable=org.eclipse.jdt.annotation.Nullable org.eclipse.jdt.core.compiler.annotation.nullable.secondary= -org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled +org.eclipse.jdt.core.compiler.annotation.nullanalysis=disabled org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled org.eclipse.jdt.core.compiler.codegen.lambda.genericSignature=do not generate org.eclipse.jdt.core.compiler.codegen.methodParameters=do not generate @@ -107,16 +107,16 @@ org.eclipse.jdt.core.compiler.problem.noImplicitStringConversion=warning org.eclipse.jdt.core.compiler.problem.nonExternalizedStringLiteral=ignore org.eclipse.jdt.core.compiler.problem.nonnullParameterAnnotationDropped=warning org.eclipse.jdt.core.compiler.problem.nonnullTypeVariableFromLegacyInvocation=warning -org.eclipse.jdt.core.compiler.problem.nullAnnotationInferenceConflict=warning +org.eclipse.jdt.core.compiler.problem.nullAnnotationInferenceConflict=error org.eclipse.jdt.core.compiler.problem.nullReference=warning -org.eclipse.jdt.core.compiler.problem.nullSpecViolation=warning +org.eclipse.jdt.core.compiler.problem.nullSpecViolation=error org.eclipse.jdt.core.compiler.problem.nullUncheckedConversion=warning org.eclipse.jdt.core.compiler.problem.overridingMethodWithoutSuperInvocation=ignore org.eclipse.jdt.core.compiler.problem.overridingPackageDefaultMethod=warning org.eclipse.jdt.core.compiler.problem.parameterAssignment=ignore org.eclipse.jdt.core.compiler.problem.pessimisticNullAnalysisForFreeTypeVariables=warning org.eclipse.jdt.core.compiler.problem.possibleAccidentalBooleanAssignment=ignore -org.eclipse.jdt.core.compiler.problem.potentialNullReference=warning +org.eclipse.jdt.core.compiler.problem.potentialNullReference=ignore org.eclipse.jdt.core.compiler.problem.potentiallyUnclosedCloseable=ignore org.eclipse.jdt.core.compiler.problem.rawTypeReference=warning org.eclipse.jdt.core.compiler.problem.redundantNullAnnotation=warning @@ -131,7 +131,7 @@ org.eclipse.jdt.core.compiler.problem.staticAccessReceiver=warning org.eclipse.jdt.core.compiler.problem.suppressOptionalErrors=disabled org.eclipse.jdt.core.compiler.problem.suppressWarnings=enabled org.eclipse.jdt.core.compiler.problem.suppressWarningsNotFullyAnalysed=info -org.eclipse.jdt.core.compiler.problem.syntacticNullAnalysisForFields=enabled +org.eclipse.jdt.core.compiler.problem.syntacticNullAnalysisForFields=disabled org.eclipse.jdt.core.compiler.problem.syntheticAccessEmulation=ignore org.eclipse.jdt.core.compiler.problem.tasks=warning org.eclipse.jdt.core.compiler.problem.terminalDeprecation=warning diff --git a/jfxui/src/main/java/org/itsallcode/whiterabbit/jfxui/ui/DailyProjectReportViewer.java b/jfxui/src/main/java/org/itsallcode/whiterabbit/jfxui/ui/DailyProjectReportViewer.java index 6ef0840d..3242b493 100644 --- a/jfxui/src/main/java/org/itsallcode/whiterabbit/jfxui/ui/DailyProjectReportViewer.java +++ b/jfxui/src/main/java/org/itsallcode/whiterabbit/jfxui/ui/DailyProjectReportViewer.java @@ -48,9 +48,9 @@ public class DailyProjectReportViewer private final Stage primaryStage; - public DailyProjectReportViewer(Stage primaryStage, UiStateService uiState, AppService appService, - UiActions uiActions, - ProjectReport report) + public DailyProjectReportViewer(final Stage primaryStage, final UiStateService uiState, final AppService appService, + final UiActions uiActions, + final ProjectReport report) { this.primaryStage = primaryStage; this.uiState = uiState; @@ -75,14 +75,14 @@ private Node[] getExportButtons() .toArray(Node[]::new); } - private Button createExportButton(AppPlugin plugin) + private Button createExportButton(final AppPlugin plugin) { final String pluginId = plugin.getId(); final EventHandler action = e -> exportReport(plugin); return UiWidget.button(pluginId + "-export-button", "Export to " + pluginId, action); } - private void exportReport(AppPlugin plugin) + private void exportReport(final AppPlugin plugin) { final var projectReportExporter = plugin.getFeature(ProjectReportExporter.class) .orElseThrow(() -> new IllegalStateException( @@ -126,7 +126,7 @@ private TreeTableView createTreeTable() return treeTable; } - private TreeItem createDayTreeItem(ProjectReportDay day) + private TreeItem createDayTreeItem(final ProjectReportDay day) { final TreeItem treeItem = new TreeItem<>(new ReportRow(day)); treeItem.setExpanded(true); @@ -146,12 +146,12 @@ public static class ReportRow private final Duration workingTime; private final String comment; - private ReportRow(ProjectReportDay day) + private ReportRow(final ProjectReportDay day) { this(day, null); } - private ReportRow(ProjectReportDay day, ProjectReportActivity project) + private ReportRow(final ProjectReportDay day, final ProjectReportActivity project) { if (project == null) { @@ -160,7 +160,7 @@ private ReportRow(ProjectReportDay day, ProjectReportActivity project) this.project = null; this.workingTime = day.getProjects().stream() .map(ProjectReportActivity::getWorkingTime) - .reduce((a, b) -> a.plus(b)) + .reduce(Duration::plus) .orElse(Duration.ZERO); this.comment = day.getComment(); } diff --git a/jfxui/src/uiTest/java/org/itsallcode/whiterabbit/jfxui/MonthlyProjectReportTest.java b/jfxui/src/uiTest/java/org/itsallcode/whiterabbit/jfxui/MonthlyProjectReportTest.java index 80ba4e72..23437159 100644 --- a/jfxui/src/uiTest/java/org/itsallcode/whiterabbit/jfxui/MonthlyProjectReportTest.java +++ b/jfxui/src/uiTest/java/org/itsallcode/whiterabbit/jfxui/MonthlyProjectReportTest.java @@ -6,9 +6,10 @@ import java.util.Locale; import org.itsallcode.whiterabbit.api.model.Project; +import org.itsallcode.whiterabbit.jfxui.testutil.JunitTags; import org.itsallcode.whiterabbit.jfxui.testutil.model.MonthlyProjectReportWindow; import org.itsallcode.whiterabbit.logic.service.project.ProjectImpl; -import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.testfx.api.FxRobot; @@ -76,7 +77,7 @@ void gotoPreviousMonth() } @Test - @Disabled("Second month change test is not working") + @Tag(JunitTags.FLAKY) // Second month change test is not working void gotoNextMonth() { time().tickSeparateMinutes(3); diff --git a/logic/src/main/java/org/itsallcode/whiterabbit/logic/model/DayActivities.java b/logic/src/main/java/org/itsallcode/whiterabbit/logic/model/DayActivities.java index 29200672..0bbd8651 100644 --- a/logic/src/main/java/org/itsallcode/whiterabbit/logic/model/DayActivities.java +++ b/logic/src/main/java/org/itsallcode/whiterabbit/logic/model/DayActivities.java @@ -27,7 +27,7 @@ public class DayActivities private final DayData day; final DayRecord dayRecord; - DayActivities(DayRecord dayRecord, ProjectService projectService, ModelFactory modelFactory) + DayActivities(final DayRecord dayRecord, final ProjectService projectService, final ModelFactory modelFactory) { this.modelFactory = modelFactory; this.day = dayRecord.getJsonDay(); @@ -75,12 +75,12 @@ private Stream getActivities() .stream(); } - private Activity wrapActivity(ActivityData activity, int index) + private Activity wrapActivity(final ActivityData activity, final int index) { return new Activity(index, activity, this, projectService); } - public Optional get(int index) + public Optional get(final int index) { return Optional.ofNullable(day.getActivities()) .filter(list -> list.size() > index) @@ -88,7 +88,7 @@ public Optional get(int index) .map(a -> wrapActivity(a, index)); } - public void remove(int index) + public void remove(final int index) { if (day.getActivities() == null) { @@ -101,7 +101,7 @@ public void remove(int index) } } - public void setRemainderActivity(ActivityData activity, boolean remainder) + public void setRemainderActivity(final ActivityData activity, final boolean remainder) { if (activity.isRemainder() == remainder) { @@ -132,7 +132,7 @@ private Duration getUnallocatedDuration() { final Duration allocatedDuration = getActivities().map(ActivityData::getDuration) .filter(Objects::nonNull) - .reduce((d1, d2) -> d1.plus(d2)) + .reduce(Duration::plus) .orElse(Duration.ZERO); return dayRecord.getWorkingTime().minus(allocatedDuration); } @@ -167,7 +167,7 @@ private List getRemainderActivities() return getActivities().filter(a -> a.getDuration() == null).collect(toList()); } - public Duration getDuration(Activity activity) + public Duration getDuration(final Activity activity) { if (activity.jsonActivity.getDuration() != null) { diff --git a/logic/src/main/java/org/itsallcode/whiterabbit/logic/model/DayRecord.java b/logic/src/main/java/org/itsallcode/whiterabbit/logic/model/DayRecord.java index dc57a34a..52c5ffc9 100644 --- a/logic/src/main/java/org/itsallcode/whiterabbit/logic/model/DayRecord.java +++ b/logic/src/main/java/org/itsallcode/whiterabbit/logic/model/DayRecord.java @@ -1,5 +1,6 @@ package org.itsallcode.whiterabbit.logic.model; +import java.time.DayOfWeek; import java.time.Duration; import java.time.LocalDate; import java.time.LocalTime; @@ -25,8 +26,9 @@ public class DayRecord implements RowRecord private final MonthIndex month; private final DayRecord previousDay; - public DayRecord(ContractTermsService contractTerms, DayData day, DayRecord previousDay, MonthIndex month, - ProjectService projectService, ModelFactory modelFactory) + public DayRecord(final ContractTermsService contractTerms, final DayData day, final DayRecord previousDay, + final MonthIndex month, + final ProjectService projectService, final ModelFactory modelFactory) { this.contractTerms = contractTerms; this.projectService = Objects.requireNonNull(projectService); @@ -111,14 +113,8 @@ public DayType getType() private boolean isWeekend() { - switch (day.getDate().getDayOfWeek()) - { - case SATURDAY: - case SUNDAY: - return true; - default: - return false; - } + final DayOfWeek dayOfWeek = day.getDate().getDayOfWeek(); + return dayOfWeek == DayOfWeek.SATURDAY || dayOfWeek == DayOfWeek.SUNDAY; } public LocalTime getBegin() @@ -126,7 +122,7 @@ public LocalTime getBegin() return day.getBegin(); } - public void setBegin(LocalTime begin) + public void setBegin(final LocalTime begin) { day.setBegin(begin); } @@ -136,7 +132,7 @@ public LocalTime getEnd() return day.getEnd(); } - public void setEnd(LocalTime end) + public void setEnd(final LocalTime end) { day.setEnd(end); } @@ -146,7 +142,7 @@ public Duration getInterruption() return day.getInterruption() == null ? Duration.ZERO : day.getInterruption(); } - public void setInterruption(Duration interruption) + public void setInterruption(final Duration interruption) { day.setInterruption(interruption.isZero() ? null : interruption); } @@ -161,12 +157,12 @@ public String getComment() return day.getComment(); } - public void setComment(String comment) + public void setComment(final String comment) { day.setComment(comment.isEmpty() ? null : comment); } - public void setType(DayType type) + public void setType(final DayType type) { day.setType(Objects.requireNonNull(type, "type")); } diff --git a/logic/src/main/java/org/itsallcode/whiterabbit/logic/report/project/ProjectReportGenerator.java b/logic/src/main/java/org/itsallcode/whiterabbit/logic/report/project/ProjectReportGenerator.java index 58a5ecf0..f06450e3 100644 --- a/logic/src/main/java/org/itsallcode/whiterabbit/logic/report/project/ProjectReportGenerator.java +++ b/logic/src/main/java/org/itsallcode/whiterabbit/logic/report/project/ProjectReportGenerator.java @@ -22,12 +22,12 @@ public class ProjectReportGenerator { private final Storage storage; - public ProjectReportGenerator(Storage storage) + public ProjectReportGenerator(final Storage storage) { this.storage = storage; } - public ProjectReport generateReport(YearMonth month) + public ProjectReport generateReport(final YearMonth month) { final List sortedDays = storage.loadMonth(month) .map(MonthIndex::getSortedDays).orElse(Stream.empty()).collect(toList()); @@ -47,7 +47,7 @@ public ProjectReport generateReport(YearMonth month) return new ProjectReportImpl(month, reportDays, reportProjects); } - private ProjectReportDay generateDayReport(DayRecord dayRecord) + private ProjectReportDay generateDayReport(final DayRecord dayRecord) { final List projects = dayRecord.activities() .getAll().stream() @@ -60,16 +60,16 @@ private ProjectReportDay generateDayReport(DayRecord dayRecord) return new DayImpl(dayRecord.getDate(), dayRecord.getType(), dayRecord.getComment(), projects); } - private String activityProject(Activity activity) + private String activityProject(final Activity activity) { return activity.getProject().getProjectId(); } - private ProjectReportImpl.ProjectActivityImpl aggregateProject(List projectActivites) + private ProjectReportImpl.ProjectActivityImpl aggregateProject(final List projectActivites) { final Duration totalWorkingTime = projectActivites.stream() .filter(activity -> activity.getDuration() != null) - .map(Activity::getDuration).reduce((a, b) -> a.plus(b)) + .map(Activity::getDuration).reduce(Duration::plus) .orElse(Duration.ZERO); final List comments = projectActivites.stream() .map(Activity::getComment) diff --git a/logic/src/main/java/org/itsallcode/whiterabbit/logic/service/singleinstance/ClientConnection.java b/logic/src/main/java/org/itsallcode/whiterabbit/logic/service/singleinstance/ClientConnection.java index 6cd58d74..b185608b 100644 --- a/logic/src/main/java/org/itsallcode/whiterabbit/logic/service/singleinstance/ClientConnection.java +++ b/logic/src/main/java/org/itsallcode/whiterabbit/logic/service/singleinstance/ClientConnection.java @@ -18,14 +18,14 @@ class ClientConnection implements AutoCloseable private final Socket clientSocket; - private ClientConnection(Socket clientSocket) + private ClientConnection(final Socket clientSocket) { this.clientSocket = clientSocket; } // Socket will be closed when client connection is closed. @SuppressWarnings("java:S2095") - static ClientConnection connect(InetAddress address, int port) + static ClientConnection connect(final InetAddress address, final int port) { try { @@ -39,7 +39,7 @@ static ClientConnection connect(InetAddress address, int port) } } - public void sendMessage(String message) + public void sendMessage(final String message) { if (message.contains("\n")) { @@ -59,7 +59,7 @@ public void sendMessage(String message) } } - public String sendMessageWithResponse(String message) + public String sendMessageWithResponse(final String message) { sendMessage(message); return readResponse(); @@ -69,12 +69,8 @@ private String readResponse() { try { - LOG.trace("Reading response..."); final BufferedReader reader = new BufferedReader(new InputStreamReader(clientSocket.getInputStream())); - - final String response = reader.readLine(); - LOG.trace("Read response '{}'", response); - return response; + return reader.readLine(); } catch (final IOException e) { diff --git a/plugins/holiday-calculator/src/test/java/org/itsallcode/whiterabbit/plugin/holidaycalculator/CalculatedHolidaysTest.java b/plugins/holiday-calculator/src/test/java/org/itsallcode/whiterabbit/plugin/holidaycalculator/CalculatedHolidaysTest.java index c111a110..67ead6d9 100644 --- a/plugins/holiday-calculator/src/test/java/org/itsallcode/whiterabbit/plugin/holidaycalculator/CalculatedHolidaysTest.java +++ b/plugins/holiday-calculator/src/test/java/org/itsallcode/whiterabbit/plugin/holidaycalculator/CalculatedHolidaysTest.java @@ -23,7 +23,7 @@ void success() throws URISyntaxException final Path path = Paths.get(this.getClass().getResource("holidays.cfg").toURI()).getParent(); final CalculatedHolidays holidays = new CalculatedHolidays(path); - assertThat(holidays.getHolidays(DATE).size()).isEqualTo(1); + assertThat(holidays.getHolidays(DATE)).hasSize(1); final HolidayInstance actual = holidays.getHolidays(DATE).get(0); assertThat(actual.getName()).isEqualTo("Fronleichnam"); assertThat(actual.getDate()).isEqualTo(DATE);