feat(#123): Program also use Source for linting#575
feat(#123): Program also use Source for linting#575Marat-Tim wants to merge 3 commits intoobjectionary:masterfrom
Conversation
🚀 Performance AnalysisAll benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information. Click to see the detailed report
|
|
@h1alexbel WDYT about this architecture? |
There was a problem hiding this comment.
@Marat-Tim see my comments, please. In general architecture is OK, the only thing concerns me is that, in Program, we depend on multiple concrete classes: Source, EoPackage. I think we can introduce more reliable dependencies via interfaces. @yegor256 WDYT?
| "passes with no exceptions", | ||
| new Source(new XMLDocument("<object/>")).defects(), | ||
| new Program( | ||
| Map.of( |
There was a problem hiding this comment.
@Marat-Tim let's use new MapOf<>() from org.cactoos for better consistency in the codebase
| /** | ||
| * A collection of lints for Whole Program Analysis (WPA), | ||
| * provided by the {@link Program} class. | ||
| * A collection of lints for Whole Package Analysis (WPA), |
There was a problem hiding this comment.
@Marat-Tim here, we should stay with "Whole Program Analysis"
|
|
||
| /** | ||
| * Whole EO package, as collection of XMIR sources to analyze. | ||
| * @since 0.1.0 |
There was a problem hiding this comment.
@Marat-Tim let's place correct version of the future release
| ) | ||
| ); | ||
|
|
||
| public class Program { |
| * <pre> | ||
| * {@code | ||
| * import com.jcabi.manifests.Manifests; | ||
| * |
There was a problem hiding this comment.
@Marat-Tim this empty line is required to separate import statement and the code
| map.putAll(Program.discover(dir)); | ||
| private static <T, E extends Exception> Collection<Source> sources(final Collection<T> elements, | ||
| final CheckedFunc<T, Source, E> ctor) throws E { | ||
| final ArrayList<Source> sources = new ArrayList<>(elements.size()); |
There was a problem hiding this comment.
@Marat-Tim can we use just java.util.Collection<Source> here, since we need only .add() and for-each?
| /** | ||
| * A single source XMIR to analyze. | ||
| * | ||
| * @see <a href="https://news.eolang.org/2022-11-25-xmir-guide.html">XMIR</a> |
There was a problem hiding this comment.
@Marat-Tim I would still leave this link, it can be useful for internal development
| @@ -1,11 +0,0 @@ | |||
| /* | |||
There was a problem hiding this comment.
@Marat-Tim what is the point of moving benchmarks to org/eolang/lints? If we make Program the only entry point to the library - we should benchmark only that class, not Source. @yegor256 WDYT?
| @Benchmark | ||
| public final void scansLargeProgram() throws IOException { | ||
| new Program(this.home).defects(); | ||
| new EoPackage(this.home).defects(); |
There was a problem hiding this comment.
@Marat-Tim here, instead of EoPackage, we should benchmark Program
| ) | ||
| ); | ||
|
|
||
| public class Program { |
There was a problem hiding this comment.
@Marat-Tim any unit tests for this class? Otherwise, we don't know what we can break making these changes
2 participants
In this pull request, I’m updating the library’s main API - now there’s a single entry point: the
ProgramclassPreviously,
Programonly searched for errors in a package, but that logic is now handled by the newEoPackageclass (copied entirely from the oldProgram)