Skip to content

8359493: Refactor how aggregated mandatory warnings are handled in the compiler #25810

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.

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 24 additions & 2 deletions make/langtools/tools/propertiesparser/gen/ClassGenerator.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -47,6 +47,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
Expand Down Expand Up @@ -93,7 +94,9 @@ enum StubKind {
FACTORY_FIELD_LINT("factory.decl.field.lint"),
WILDCARDS_EXTENDS("wildcards.extends"),
SUPPRESS_WARNINGS("suppress.warnings"),
LINT_CATEGORY("lint.category");
LINT_CATEGORY("lint.category"),
DIAGNOSTIC_FLAGS_EMPTY("diagnostic.flags.empty"),
DIAGNOSTIC_FLAGS_NON_EMPTY("diagnostic.flags.non-empty");

/** stub key (as it appears in the property file) */
String key;
Expand Down Expand Up @@ -259,17 +262,30 @@ List<String> generateFactoryMethodsAndFields(FactoryKind k, String key, Message
.map(MessageLine::lintCategory)
.findFirst().orElse(null);
//System.out.println("category for " + key + " = " + lintCategory);
String diagnosticFlags = lines.stream()
.filter(MessageLine::isDiagnosticFlags)
.map(MessageLine::diagnosticFlags)
.flatMap(Stream::of)
.map(s -> s.replace('-', '_'))
.map(s -> s.toUpperCase(Locale.ROOT))
.collect(Collectors.joining(", "));
String factoryName = factoryName(key);
if (msgInfo.getTypes().isEmpty()) {
//generate field
String factoryField;
if (lintCategory == null) {
factoryField = StubKind.FACTORY_FIELD.format(k.keyClazz, factoryName,
diagnosticFlags.isEmpty() ?
StubKind.DIAGNOSTIC_FLAGS_EMPTY.format() :
StubKind.DIAGNOSTIC_FLAGS_NON_EMPTY.format(diagnosticFlags),
"\"" + keyParts[0] + "\"",
"\"" + Stream.of(keyParts).skip(2).collect(Collectors.joining(".")) + "\"",
javadoc);
} else {
factoryField = StubKind.FACTORY_FIELD_LINT.format(k.keyClazz, factoryName,
diagnosticFlags.isEmpty() ?
StubKind.DIAGNOSTIC_FLAGS_EMPTY.format() :
StubKind.DIAGNOSTIC_FLAGS_NON_EMPTY.format(diagnosticFlags),
StubKind.LINT_CATEGORY.format("\"" + lintCategory + "\""),
"\"" + keyParts[0] + "\"",
"\"" + Stream.of(keyParts).skip(2).collect(Collectors.joining(".")) + "\"",
Expand All @@ -287,11 +303,17 @@ List<String> generateFactoryMethodsAndFields(FactoryKind k, String key, Message
String methodBody;
if (lintCategory == null) {
methodBody = StubKind.FACTORY_METHOD_BODY.format(k.keyClazz,
diagnosticFlags.isEmpty() ?
StubKind.DIAGNOSTIC_FLAGS_EMPTY.format() :
StubKind.DIAGNOSTIC_FLAGS_NON_EMPTY.format(diagnosticFlags),
"\"" + keyParts[0] + "\"",
"\"" + Stream.of(keyParts).skip(2).collect(Collectors.joining(".")) + "\"",
argNames.stream().collect(Collectors.joining(", ")));
} else {
methodBody = StubKind.FACTORY_METHOD_BODY_LINT.format(k.keyClazz,
diagnosticFlags.isEmpty() ?
StubKind.DIAGNOSTIC_FLAGS_EMPTY.format() :
StubKind.DIAGNOSTIC_FLAGS_NON_EMPTY.format(diagnosticFlags),
StubKind.LINT_CATEGORY.format("\"" + lintCategory + "\""),
"\"" + keyParts[0] + "\"",
"\"" + Stream.of(keyParts).skip(2).collect(Collectors.joining(".")) + "\"",
Expand Down
7 changes: 4 additions & 3 deletions make/langtools/tools/propertiesparser/parser/Message.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
* A message within the message file.
* A message is a series of lines containing a "name=value" property,
* optionally preceded by a comment describing the use of placeholders
* such as {0}, {1}, etc within the property value.
* such as {0}, {1}, etc within the property value, a lint category,
* and/or a list of diagnostic flags.
*/
public final class Message {
final MessageLine firstLine;
Expand All @@ -49,7 +50,7 @@ public final class Message {
public MessageInfo getMessageInfo() {
if (messageInfo == null) {
MessageLine l = firstLine.prev;
if (l != null && l.isLint()) {
while (l != null && (l.isLint() || l.isDiagnosticFlags())) {
l = l.prev;
}
if (l != null && l.isInfo())
Expand All @@ -74,7 +75,7 @@ public List<MessageLine> getLines(boolean includeAllPrecedingComments) {
while (l.text.isEmpty())
l = l.next;
} else {
if (l.prev != null && (l.prev.isInfo() || l.prev.isLint()))
while (l.prev != null && (l.prev.isInfo() || l.prev.isLint() || l.prev.isDiagnosticFlags()))
l = l.prev;
}

Expand Down
17 changes: 16 additions & 1 deletion make/langtools/tools/propertiesparser/parser/MessageLine.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -25,6 +25,7 @@

package propertiesparser.parser;

import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand All @@ -39,6 +40,7 @@ public class MessageLine {
static final Pattern infoPattern = Pattern.compile(String.format("# ([0-9]+: %s, )*[0-9]+: %s",
typePattern.pattern(), typePattern.pattern()));
static final Pattern lintPattern = Pattern.compile("# lint: ([a-z\\-]+)");
static final Pattern diagnosticFlagsPattern = Pattern.compile("# flags: ([a-z\\-]+(, ([a-z\\-]+))*)");

public String text;
MessageLine prev;
Expand Down Expand Up @@ -69,6 +71,19 @@ public String lintCategory() {
}
}

public boolean isDiagnosticFlags() {
return diagnosticFlagsPattern.matcher(text).matches();
}

public String[] diagnosticFlags() {
Matcher matcher = diagnosticFlagsPattern.matcher(text);
if (matcher.matches()) {
return matcher.group(1).split(", ", -1);
} else {
return null;
}
}

boolean hasContinuation() {
return (next != null) && text.endsWith("\\");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved.
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
#
# This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -27,13 +27,18 @@ toplevel.decl=\
package {0};\n\
\n\
{1}\n\
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticFlag;\n\
import com.sun.tools.javac.util.JCDiagnostic.Error;\n\
import com.sun.tools.javac.util.JCDiagnostic.Warning;\n\
import com.sun.tools.javac.util.JCDiagnostic.LintWarning;\n\
import com.sun.tools.javac.util.JCDiagnostic.Note;\n\
import com.sun.tools.javac.util.JCDiagnostic.Fragment;\n\
import com.sun.tools.javac.code.Lint.LintCategory;\n\
\n\
import java.util.EnumSet;\n\
\n\
import static com.sun.tools.javac.util.JCDiagnostic.DiagnosticFlag.*;\n\
\n\
public class {2} '{'\n\
{3}\n\
'}'\n
Expand All @@ -58,22 +63,22 @@ factory.decl.method.arg=\
arg{0}

factory.decl.method.body=\
return new {0}({1}, {2}, {3});
return new {0}({1}, {2}, {3}, {4});

factory.decl.method.body.lint=\
return new {0}({1}, {2}, {3}, {4});
return new {0}({1}, {2}, {3}, {4}, {5});

factory.decl.field=\
/**\n\
' '* {4}\n\
' '*/\n\
public static final {0} {1} = new {0}({2}, {3});
public static final {0} {1} = new {0}({2}, {3}, {4});

factory.decl.field.lint=\
/**\n\
' '* {5}\n\
' '*/\n\
public static final {0} {1} = new {0}({2}, {3}, {4});
public static final {0} {1} = new {0}({2}, {3}, {4}, {5});

wildcards.extends=\
{0}<? extends {1}>
Expand All @@ -84,3 +89,9 @@ suppress.warnings=\
lint.category=\
LintCategory.get({0}).get()

diagnostic.flags.empty=\
EnumSet.noneOf(DiagnosticFlag.class)

diagnostic.flags.non-empty=\
EnumSet.of({0})

Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,6 @@ void clear() {
((ReusableJavaCompiler)ReusableJavaCompiler.instance(this)).clear();
Types.instance(this).newRound();
Check.instance(this).newRound();
Check.instance(this).clear(); //clear mandatory warning handlers
Preview.instance(this).clear(); //clear mandatory warning handlers
Modules.instance(this).newRound();
Annotate.instance(this).newRound();
CompileStates.instance(this).clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import com.sun.tools.javac.util.JCDiagnostic.SimpleDiagnosticPosition;
import com.sun.tools.javac.util.JCDiagnostic.Warning;
import com.sun.tools.javac.util.Log;
import com.sun.tools.javac.util.MandatoryWarningHandler;
import com.sun.tools.javac.util.Names;
import com.sun.tools.javac.util.Options;

Expand Down Expand Up @@ -71,9 +70,6 @@ public class Preview {
/** flag: is the "preview" lint category enabled? */
private final boolean verbose;

/** the diag handler to manage preview feature usage diagnostics */
private final MandatoryWarningHandler previewHandler;

/** test flag: should all features be considered as preview features? */
private final boolean forcePreview;

Expand Down Expand Up @@ -105,7 +101,6 @@ protected Preview(Context context) {
log = Log.instance(context);
source = Source.instance(context);
verbose = Lint.instance(context).isEnabled(LintCategory.PREVIEW);
previewHandler = new MandatoryWarningHandler(log, source, verbose, true, LintCategory.PREVIEW);
forcePreview = options.isSet("forcePreview");
majorVersionToSource = initMajorVersionToSourceMap();
}
Expand Down Expand Up @@ -176,7 +171,8 @@ public void warnPreview(DiagnosticPosition pos, Feature feature) {
Assert.check(isEnabled());
Assert.check(isPreview(feature));
markUsesPreview(pos);
previewHandler.report(pos, feature.isPlural() ?
log.mandatoryWarning(pos,
feature.isPlural() ?
LintWarnings.PreviewFeatureUsePlural(feature.nameFragment()) :
LintWarnings.PreviewFeatureUse(feature.nameFragment()));
}
Expand All @@ -203,10 +199,6 @@ public void markUsesPreview(DiagnosticPosition pos) {
sourcesWithPreviewFeatures.add(log.currentSourceFile());
}

public void reportPreviewWarning(DiagnosticPosition pos, LintWarning warnKey) {
previewHandler.report(pos, warnKey);
}

public boolean usesPreview(JavaFileObject file) {
return sourcesWithPreviewFeatures.contains(file);
}
Expand Down Expand Up @@ -269,25 +261,13 @@ public boolean declaredUsingPreviewFeature(Symbol sym) {
return false;
}

/**
* Report any deferred diagnostics.
*/
public void reportDeferredDiagnostics() {
previewHandler.reportDeferredDiagnostic();
}

public void clear() {
previewHandler.clear();
}

public void checkSourceLevel(DiagnosticPosition pos, Feature feature) {
if (isPreview(feature) && !isEnabled()) {
//preview feature without --preview flag, error
log.error(JCDiagnostic.DiagnosticFlag.SOURCE_LEVEL, pos, disabledError(feature));
log.error(pos, disabledError(feature));
} else {
if (!feature.allowedInSource(source)) {
log.error(JCDiagnostic.DiagnosticFlag.SOURCE_LEVEL, pos,
feature.error(source.name));
log.error(pos, feature.error(source.name));
}
if (isEnabled() && isPreview(feature)) {
warnPreview(pos, feature);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
import static com.sun.tools.javac.code.TypeTag.*;
import static com.sun.tools.javac.code.TypeTag.WILDCARD;
import static com.sun.tools.javac.tree.JCTree.Tag.*;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticFlag;

/** This is the main context-dependent analysis phase in GJC. It
* encompasses name resolution, type checking and constant folding as
Expand Down Expand Up @@ -4146,8 +4145,7 @@ public void visitTypeTest(JCInstanceOf tree) {
!exprtype.isErroneous() && !clazztype.isErroneous() &&
tree.pattern.getTag() != RECORDPATTERN) {
if (!allowUnconditionalPatternsInstanceOf) {
log.error(DiagnosticFlag.SOURCE_LEVEL, tree.pos(),
Feature.UNCONDITIONAL_PATTERN_IN_INSTANCEOF.error(this.sourceName));
log.error(tree.pos(), Feature.UNCONDITIONAL_PATTERN_IN_INSTANCEOF.error(this.sourceName));
}
}
typeTree = TreeInfo.primaryPatternTypeTree((JCPattern) tree.pattern);
Expand All @@ -4167,8 +4165,7 @@ public void visitTypeTest(JCInstanceOf tree) {
if (allowReifiableTypesInInstanceof) {
valid = checkCastablePattern(tree.expr.pos(), exprtype, clazztype);
} else {
log.error(DiagnosticFlag.SOURCE_LEVEL, tree.pos(),
Feature.REIFIABLE_TYPES_INSTANCEOF.error(this.sourceName));
log.error(tree.pos(), Feature.REIFIABLE_TYPES_INSTANCEOF.error(this.sourceName));
allowReifiableTypesInInstanceof = true;
}
if (!valid) {
Expand Down
Loading