Skip to content

refactor(#4729): move pinfos method to new PackageInfos class #4797

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

Merged
yegor256 merged 1 commit into from
Jan 13, 2026
Merged

refactor(#4729): move pinfos method to new PackageInfos class #4797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
57 changes: 1 addition & 56 deletions eo-maven-plugin/src/main/java/org/eolang/maven/MjTranspile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,14 @@
import com.yegor256.xsline.TrJoined;
import com.yegor256.xsline.Train;
import com.yegor256.xsline.Xsline;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import org.apache.maven.plugins.annotations.LifecyclePhase;
import org.apache.maven.plugins.annotations.Mojo;
Expand Down Expand Up @@ -70,11 +67,6 @@ public final class MjTranspile extends MjSafe {
*/
private static final String JAVA = "java";

/**
* Pattern for replacing EO in package.
*/
private static final Pattern PACKAGE = Pattern.compile("EO");

/**
* Parsing train with XSLs.
*/
Expand Down Expand Up @@ -123,7 +115,7 @@ public void exec() throws IOException {
final int saved = new Threaded<>(
sources,
this::transpiled
).total() + MjTranspile.pinfos(this.generatedDir.toPath());
).total() + new PackageInfos(this.generatedDir.toPath()).create();
Logger.info(
this, "Transpiled %d XMIRs, created %d Java files in %[file]s",
sources.size(), saved, this.generatedDir
Expand Down Expand Up @@ -253,53 +245,6 @@ clazz, new FileGenerationReport(saved, tgt, target)
return saved.get();
}

/**
* Create {@code package-info.java} files in all the directories
* in {@link MjTranspile#generatedDir}.
* @param generated Path to generated sources
* @return Amount of created files
* @throws IOException If fails to create a file
* @todo #4717:90min Move {@link #pinfos(Path)} method to a separate class.
* Currently, this method violates Single Responsibility Principle of
* MjTranspile class. After moving, make sure to cover it with unit tests.
*/
private static int pinfos(final Path generated) throws IOException {
final int size;
if (Files.exists(generated)) {
final List<Path> dirs = Files.walk(generated)
.filter(file -> Files.isDirectory(file) && !file.equals(generated))
.collect(Collectors.toList());
for (final Path dir : dirs) {
final String pkg = generated.relativize(dir).toString()
.replace(File.separator, ".");
final Path saved = new Saved(
String.join(
"\n",
"/**",
" * This file was auto-generated by eo-maven-plugin,",
" * don't modify it, all changes will be lost anyway.",
" */",
String.format(
"// @org.eolang.XmirPackage(\"%s\")",
MjTranspile.PACKAGE.matcher(pkg).replaceAll("")
),
String.format("package %s;", pkg)
),
dir.resolve("package-info.java")
).value();
Logger.debug(MjTranspile.class, "Created %s", saved);
}
size = dirs.size();
} else {
Logger.info(
MjTranspile.class,
"No generated sources found, skipping package-info.java creation"
);
size = 0;
}
return size;
}

/**
* Cached path.
* @param hsh Hash
Expand Down
84 changes: 84 additions & 0 deletions eo-maven-plugin/src/main/java/org/eolang/maven/PackageInfos.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2016-2026 Objectionary.com
* SPDX-License-Identifier: MIT
*/
package org.eolang.maven;

import com.jcabi.log.Logger;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

/**
* Package info classes.
* @since 0.60
*/
final class PackageInfos {

/**
* Pattern for replacing EO in package.
*/
private static final Pattern PACKAGE = Pattern.compile("EO");

/**
* Directory where create package info files.
*/
private final Path root;

/**
* Constructor.
* @param root In which directory create files.
*/
PackageInfos(final Path root) {
this.root = root;
}

/**
* Create {@code package-info.java} files in all the directories under the {@link #root}.
* @return Amount of created files
* @throws IOException If fails to create a file
*/
int create() throws IOException {
final int size;
if (Files.exists(this.root)) {
final List<Path> dirs = Files.walk(this.root)
.filter(file -> Files.isDirectory(file) && !file.equals(this.root))
.collect(Collectors.toList());
Comment on lines +48 to +50

@coderabbitai coderabbitai Bot Jan 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Resource leak: Files.walk() stream is not closed.

Files.walk() returns a Stream<Path> backed by I/O resources that must be explicitly closed. Using .collect() does not close the underlying stream, which can lead to file handle exhaustion.

🔧 Proposed fix using try-with-resources 
         if (Files.exists(this.root)) {
-            final List<Path> dirs = Files.walk(this.root)
-                .filter(file -> Files.isDirectory(file) && !file.equals(this.root))
-                .collect(Collectors.toList());
+            final List<Path> dirs;
+            try (java.util.stream.Stream<Path> stream = Files.walk(this.root)) {
+                dirs = stream
+                    .filter(file -> Files.isDirectory(file) && !file.equals(this.root))
+                    .collect(Collectors.toList());
+            }
             for (final Path dir : dirs) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
final List<Path> dirs = Files.walk(this.root)
.filter(file -> Files.isDirectory(file) && !file.equals(this.root))
.collect(Collectors.toList());
final List<Path> dirs;
try (java.util.stream.Stream<Path> stream = Files.walk(this.root)) {
dirs = stream
.filter(file -> Files.isDirectory(file) && !file.equals(this.root))
.collect(Collectors.toList());
}
🤖 Prompt for AI Agents 
In @eo-maven-plugin/src/main/java/org/eolang/maven/PackageInfos.java around
lines 48 - 50, The Files.walk(this.root) stream in PackageInfos is not closed;
change the code that builds dirs to open the Stream<Path> in a
try-with-resources (e.g., try (Stream<Path> walk = Files.walk(this.root)) { dirs
= walk.filter(...).collect(...) }) so the stream is automatically closed; update
the declaration of dirs (and any surrounding scope) so the collected List<Path>
is assigned inside the try block and available afterward.

Comment on lines +48 to +50

Copilot AI Jan 13, 2026

Copy link

Choose a reason for hiding this comment

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

The variable name "dirs" is a compound abbreviation. According to the project guidelines, compound variable names should be avoided in favor of single-word nouns. Consider renaming to "directories" or simply "paths" to follow the naming convention.

Copilot generated this review using guidance from organization custom instructions.
for (final Path dir : dirs) {
final String pkg = this.root.relativize(dir).toString()
.replace(File.separator, ".");
Comment on lines +52 to +53

Copilot AI Jan 13, 2026

Copy link

Choose a reason for hiding this comment

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

The variable name "pkg" is an abbreviation. According to the project guidelines, compound or abbreviated variable names should be avoided in favor of single-word nouns. Consider renaming to "package" (though this is a Java keyword, so "name" might be a better alternative).

Copilot generated this review using guidance from organization custom instructions.
final Path saved = new Saved(
PackageInfos.content(pkg), dir.resolve("package-info.java")
).value();
Logger.debug(this, "Created %s", saved);
}
size = dirs.size();
} else {
Logger.info(
this,
"No generated sources found, skipping package-info.java creation"
);
size = 0;
}
return size;
}

private static String content(final String pkg) {
return String.join(
"\n",
"/**",
" * This file was auto-generated by eo-maven-plugin,",
" * don't modify it, all changes will be lost anyway.",
" */",
String.format(
"// @org.eolang.XmirPackage(\"%s\")",
PackageInfos.PACKAGE.matcher(pkg).replaceAll("")
),
String.format("package %s;", pkg)
);
}
}
1 change: 0 additions & 1 deletion eo-maven-plugin/src/test/java/org/eolang/maven/MjTranspileTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ void createsPackageInfoFilesForAllPackages(@Mktmp final Path temp) throws IOExce
);
}

@Disabled
@Test
void savesValidContentToPackageInfoFile(@Mktmp final Path temp) throws Exception {
MatcherAssert.assertThat(
Expand Down
56 changes: 56 additions & 0 deletions eo-maven-plugin/src/test/java/org/eolang/maven/PackageInfosTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2016-2026 Objectionary.com
* SPDX-License-Identifier: MIT
*/
package org.eolang.maven;

import com.yegor256.Mktmp;
import com.yegor256.MktmpResolver;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

/**
* Test cases for {@link PackageInfos}.
* @since 0.60
*/
@ExtendWith(MktmpResolver.class)
final class PackageInfosTest {

@Test
void createsPackageInfosInSubDirectories(@Mktmp final Path tmp) throws IOException {
final Path subdir = tmp.resolve("subdir");
final Path subsubdir = subdir.resolve("subsubdir");
Files.createDirectory(subdir);
Files.createDirectories(subsubdir);
MatcherAssert.assertThat(
"We should create exactly two package-info.java files for two subdirectories",
new PackageInfos(tmp).create(),
Matchers.equalTo(2)
);
MatcherAssert.assertThat(
"package-info.java should be created in the both subdirectories",
Files.exists(subdir.resolve("package-info.java"))
&& Files.exists(subsubdir.resolve("package-info.java")),
Matchers.is(true)
);
Comment on lines +30 to +40

Copilot AI Jan 13, 2026

Copy link

Choose a reason for hiding this comment

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

This test contains two assertions, which violates the project guideline that each test should contain only one assertion. Consider splitting this into two separate tests: one to verify the count of created files, and another to verify the existence of package-info.java files in subdirectories.

Copilot generated this review using guidance from organization custom instructions.
}

@Test
void ignoresTheRootDirectoryItself(@Mktmp final Path tmp) throws IOException {
MatcherAssert.assertThat(
"No package-info.java files should be created in the root directory",
new PackageInfos(tmp).create(),
Matchers.equalTo(0)
);
MatcherAssert.assertThat(
"package-info.java should not be created in the root directory",
Files.exists(tmp.resolve("package-info.java")),
Matchers.is(false)
);
Comment on lines +45 to +54

Copilot AI Jan 13, 2026

Copy link

Choose a reason for hiding this comment

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

This test contains two assertions, which violates the project guideline that each test should contain only one assertion. Consider splitting this into two separate tests: one to verify the count of created files is 0, and another to verify package-info.java does not exist in the root directory.

Copilot generated this review using guidance from organization custom instructions.
}
}
Loading