RFR: 8136583: Core libraries should use blessed modifier order

[Martin Buchholz]

I love both hacky regexy perl and AST-based super-correct javaey analysis,
and I welcome the competition!  My hacky script knows its limits and is not
going to try to get e.g. enclosing class vs. interface correct.

On Wed, Sep 16, 2015 at 11:49 AM, joe darcy <joe.darcy at oracle.com> wrote:

> FWIW, for a related possible future cleanup some modifiers by convention
> can be omitted, such as abstract on normal interface methods. The
> printModifiers method written annotation processing
>
>
> http://hg.openjdk.java.net/jdk9/dev/langtools/file/286fc9270404/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/PrintingProcessor.java
>
> tries to output the informative modifiers (in the suggested order).
>
> -Joe
>
> On 9/16/2015 11:20 AM, Martin Buchholz wrote:
>
>> Peter, that Java program is awesome, but ...
>> the original perl seems more readable to me (!)
>> even if I apply some not-yet-done perl golf to it,
>> and it seems to me that if we're rewriting in Java we can get the benefits
>> of Java, i.e. correctness, which you can probably get by tapping into
>> javac's conversion to AST.  This would allow you to get modifiers split
>> across lines correct, for example.
>>
>> In practice we want to change the order within e.g. javadoc comments as
>> well, but we'll leave that to a bolder future code janitor.
>>
>> On Wed, Sep 16, 2015 at 8:19 AM, Peter Levart <peter.levart at gmail.com>
>> wrote:
>>
>> Hi Martin,
>>>
>>>
>>> On 09/16/2015 02:42 AM, Martin Buchholz wrote:
>>>
>>> Hi, Chris and Paul,
>>> I'd like you to do a very boring code review.
>>>
>>> This change is entirely machine generated. (the script is more
>>> interesting)
>>> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/blessed-modifier-order/blessed-modifier-order.patchhttps://bugs.openjdk.java.net/browse/JDK-8136583
>>>
>>>
>>>
>>> Reviewing the diff would be boring, yes. More interesting is trying to
>>> understand the script.
>>>
>>> I re-implemented your script in some other language (called Java ;-) and
>>> compared the results. Nothing can beat concise expressiveness of bash
>>> instantiations of perl template, but Java lambdas and streams are not too
>>> shabby either. Here's a re-implementation in 2 expressions and 1
>>> statement:
>>>
>>> import java.io.IOException;
>>> import java.io.UncheckedIOException;
>>> import java.nio.file.Files;
>>> import java.nio.file.Path;
>>> import java.nio.file.Paths;
>>> import java.util.Arrays;
>>> import java.util.List;
>>> import java.util.function.Function;
>>> import java.util.regex.Pattern;
>>> import java.util.stream.Collectors;
>>> import java.util.stream.IntStream;
>>> import java.util.stream.Stream;
>>>
>>> import static java.nio.file.StandardCopyOption.*;
>>> import static java.nio.file.StandardOpenOption.*;
>>>
>>> public class BlessedModifierOrder {
>>>
>>>      static final List<String> modifiers = Arrays.asList(
>>>          "public", "protected", "private",
>>>          "abstract", "static", "final", "transient",
>>>          "volatile", "synchronized", "native", "strictfp"
>>>      );
>>>
>>>      static final Function<String, String> editor = IntStream
>>>          .range(3, modifiers.size())
>>>          .mapToObj(i -> Pattern.compile(
>>>              "^([A-Za-z@ ]*)\\b(" +
>>>              modifiers.subList(i,
>>> modifiers.size()).stream().collect(Collectors.joining("|")) +
>>>              ") +(" +
>>>              modifiers.subList(0,
>>> i).stream().collect(Collectors.joining("|")) +
>>>              ")\\b"
>>>          ))
>>>          .map(pattern -> (Function<String, String>) line -> {
>>>              String editedLine;
>>>              while (!line.equals(editedLine =
>>> pattern.matcher(line).replaceFirst("$1$3 $2"))) {
>>>                  line = editedLine;
>>>              }
>>>              return line;
>>>          })
>>>          .reduce(Function.identity(), (f1, f2) -> f1.andThen(f2));
>>>
>>>      public static void main(String[] args) {
>>>          Stream.of(args)
>>>                .map(Paths::get)
>>>                .flatMap(dir -> {
>>>                    try (Stream<Path> files = Files.find(
>>>                        dir, Integer.MAX_VALUE,
>>>                        (p, attr) -> attr.isRegularFile() &&
>>> p.toString().endsWith(".java")
>>>                    )) {
>>>                        return
>>> files.collect(Collectors.toList()).stream();
>>>                    } catch (IOException e) {
>>>                        throw new UncheckedIOException(e);
>>>                    }
>>>                })
>>>                .forEach(file -> {
>>>                    try (Stream<String> linesStream = Files.lines(file)) {
>>>                        List<String> lines = linesStream.map(editor)
>>>
>>> .collect(Collectors.toList());
>>>                        Path newFile =
>>> file.resolveSibling(file.getFileName() + ".new");
>>>                        Files.write(newFile, lines, WRITE, CREATE,
>>> TRUNCATE_EXISTING);
>>>                        Files.move(newFile, file, ATOMIC_MOVE,
>>> REPLACE_EXISTING);
>>>                    } catch (IOException e) {
>>>                        throw new UncheckedIOException(e);
>>>                    }
>>>                });
>>>      }
>>> }
>>>
>>>
>>> Anyway, the result is exactly the same.
>>>
>>>
>>> Regards, Peter
>>>
>>>
>>>
>