Skip to content

Commit

Permalink
Separate LintWarnings from Warnings in CompilerProperties
Browse files Browse the repository at this point in the history 
Add Lint::logIfEnabled
  • Loading branch information
@mcimadamore
mcimadamore committed Dec 4, 2024
1 parent 06df4f7 commit c49f7a5
Show file tree
Hide file tree
Showing 21 changed files with 225 additions and 197 deletions.
26 changes: 19 additions & 7 deletions make/langtools/tools/propertiesparser/gen/ClassGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
import java.util.Set;
import java.util.TreeMap;
Expand Down Expand Up @@ -117,6 +118,7 @@ String format(Object... args) {
enum FactoryKind {
ERR("err", "Error", "Errors"),
WARN("warn", "Warning", "Warnings"),
LINT_WARN("warn", "LintWarning", "LintWarnings"),
NOTE("note", "Note", "Notes"),
MISC("misc", "Fragment", "Fragments"),
OTHER(null, null, null);
Expand All @@ -139,13 +141,24 @@ enum FactoryKind {
/**
* Utility method for parsing a factory kind from a resource key prefix.
*/
static FactoryKind parseFrom(String prefix) {
static FactoryKind of(Entry<String, Message> messageEntry) {
String prefix = messageEntry.getKey().split("\\.")[1];
FactoryKind selected = null;
for (FactoryKind k : FactoryKind.values()) {
if (k.prefix == null || k.prefix.equals(prefix)) {
return k;
selected = k;
break;
}
}
return null;
if (selected == WARN) {
for (MessageLine line : messageEntry.getValue().getLines(false)) {
if (line.isLint()) {
selected = LINT_WARN;
break;
}
}
}
return selected;
}
}

Expand All @@ -158,7 +171,7 @@ public void generateFactory(MessageFile messageFile, File outDir) {
messageFile.messages.entrySet().stream()
.collect(
Collectors.groupingBy(
e -> FactoryKind.parseFrom(e.getKey().split("\\.")[1]),
FactoryKind::of,
TreeMap::new,
toList()));
//generate nested classes
Expand All @@ -168,7 +181,7 @@ public void generateFactory(MessageFile messageFile, File outDir) {
if (entry.getKey() == FactoryKind.OTHER) continue;
//emit members
String members = entry.getValue().stream()
.flatMap(e -> generateFactoryMethodsAndFields(e.getKey(), e.getValue()).stream())
.flatMap(e -> generateFactoryMethodsAndFields(entry.getKey(), e.getKey(), e.getValue()).stream())
.collect(Collectors.joining("\n\n"));
//emit nested class
String factoryDecl =
Expand Down Expand Up @@ -233,15 +246,14 @@ List<String> generateImports(Set<String> importedTypes) {
/**
* Generate a list of factory methods/fields to be added to a given factory nested class.
*/
List<String> generateFactoryMethodsAndFields(String key, Message msg) {
List<String> generateFactoryMethodsAndFields(FactoryKind k, String key, Message msg) {
MessageInfo msgInfo = msg.getMessageInfo();
List<MessageLine> lines = msg.getLines(false);
String javadoc = lines.stream()
.filter(ml -> !ml.isInfo() && !ml.isEmptyOrComment())
.map(ml -> ml.text)
.collect(Collectors.joining("\n *"));
String[] keyParts = key.split("\\.");
FactoryKind k = FactoryKind.parseFrom(keyParts[1]);
String lintCategory = lines.stream()
.filter(MessageLine::isLint)
.map(MessageLine::lintCategory)
Expand Down
1 change: 1 addition & 0 deletions make/langtools/tools/propertiesparser/resources/templates.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ toplevel.decl=\
{1}\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\
Expand Down
15 changes: 15 additions & 0 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@
import com.sun.tools.javac.code.Symbol.*;
import com.sun.tools.javac.main.Option;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
import com.sun.tools.javac.util.JCDiagnostic.LintWarning;
import com.sun.tools.javac.util.List;
import com.sun.tools.javac.util.Log;
import com.sun.tools.javac.util.Options;
import com.sun.tools.javac.util.Pair;

Expand Down Expand Up @@ -93,6 +96,7 @@ public Lint suppress(LintCategory... lc) {
return l;
}

private final Log log;
private final AugmentVisitor augmentor;

private final EnumSet<LintCategory> values;
Expand Down Expand Up @@ -146,12 +150,14 @@ protected Lint(Context context) {

context.put(lintKey, this);
augmentor = new AugmentVisitor(context);
log = Log.instance(context);
}

protected Lint(Lint other) {
this.augmentor = other.augmentor;
this.values = other.values.clone();
this.suppressedValues = other.suppressedValues.clone();
this.log = other.log;
}

@Override
Expand Down Expand Up @@ -385,6 +391,15 @@ public boolean isSuppressed(LintCategory lc) {
return suppressedValues.contains(lc);
}

/**
* Helper method. Log a lint warning if its lint category is enabled.
*/
public void logIfEnabled(DiagnosticPosition pos, LintWarning warning) {
if (isEnabled(warning.getLintCategory())) {
log.warning(pos, warning);
}
}

protected static class AugmentVisitor implements Attribute.Visitor {
private final Context context;
private Symtab syms;
Expand Down
12 changes: 7 additions & 5 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@
import com.sun.tools.javac.code.Symbol.ModuleSymbol;
import com.sun.tools.javac.jvm.Target;
import com.sun.tools.javac.resources.CompilerProperties.Errors;
import com.sun.tools.javac.resources.CompilerProperties.LintWarnings;
import com.sun.tools.javac.resources.CompilerProperties.Warnings;
import com.sun.tools.javac.util.Assert;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
import com.sun.tools.javac.util.JCDiagnostic.Error;
import com.sun.tools.javac.util.JCDiagnostic.LintWarning;
import com.sun.tools.javac.util.JCDiagnostic.SimpleDiagnosticPosition;
import com.sun.tools.javac.util.JCDiagnostic.Warning;
import com.sun.tools.javac.util.Log;
Expand Down Expand Up @@ -102,7 +104,7 @@ protected Preview(Context context) {
lint = Lint.instance(context);
source = Source.instance(context);
this.previewHandler =
new MandatoryWarningHandler(log, source, lint.isEnabled(LintCategory.PREVIEW), true, "preview");
new MandatoryWarningHandler(log, source, lint.isEnabled(LintCategory.PREVIEW), true, "preview", LintCategory.PREVIEW);
forcePreview = options.isSet("forcePreview");
majorVersionToSource = initMajorVersionToSourceMap();
}
Expand Down Expand Up @@ -175,8 +177,8 @@ public void warnPreview(DiagnosticPosition pos, Feature feature) {
if (!lint.isSuppressed(LintCategory.PREVIEW)) {
sourcesWithPreviewFeatures.add(log.currentSourceFile());
previewHandler.report(pos, feature.isPlural() ?
Warnings.PreviewFeatureUsePlural(feature.nameFragment()) :
Warnings.PreviewFeatureUse(feature.nameFragment()));
LintWarnings.PreviewFeatureUsePlural(feature.nameFragment()) :
LintWarnings.PreviewFeatureUse(feature.nameFragment()));
}
}

Expand All @@ -189,15 +191,15 @@ public void warnPreview(JavaFileObject classfile, int majorVersion) {
Assert.check(isEnabled());
if (lint.isEnabled(LintCategory.PREVIEW)) {
log.mandatoryWarning(null,
Warnings.PreviewFeatureUseClassfile(classfile, majorVersionToSource.get(majorVersion).name));
LintWarnings.PreviewFeatureUseClassfile(classfile, majorVersionToSource.get(majorVersion).name));
}
}

public void markUsesPreview(DiagnosticPosition pos) {
sourcesWithPreviewFeatures.add(log.currentSourceFile());
}

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

Expand Down
25 changes: 12 additions & 13 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@

import com.sun.tools.javac.resources.CompilerProperties.Errors;
import com.sun.tools.javac.resources.CompilerProperties.Fragments;
import com.sun.tools.javac.resources.CompilerProperties.LintWarnings;
import com.sun.tools.javac.resources.CompilerProperties.Warnings;
import com.sun.tools.javac.tree.*;
import com.sun.tools.javac.tree.JCTree.*;
Expand Down Expand Up @@ -1938,8 +1939,8 @@ private Symbol enumConstant(JCTree tree, Type enumType) {

public void visitSynchronized(JCSynchronized tree) {
chk.checkRefType(tree.pos(), attribExpr(tree.lock, env));
if (env.info.lint.isEnabled(LintCategory.SYNCHRONIZATION) && isValueBased(tree.lock.type)) {
log.warning(tree.pos(), Warnings.AttemptToSynchronizeOnInstanceOfValueBasedClass);
if (isValueBased(tree.lock.type)) {
env.info.lint.logIfEnabled(tree.pos(), LintWarnings.AttemptToSynchronizeOnInstanceOfValueBasedClass);
}
attribStat(tree.body, env);
result = null;
Expand Down Expand Up @@ -2045,9 +2046,8 @@ void checkAutoCloseable(DiagnosticPosition pos, Env<AttrContext> env, Type resou
}
if (close.kind == MTH &&
close.overrides(syms.autoCloseableClose, resource.tsym, types, true) &&
chk.isHandled(syms.interruptedExceptionType, types.memberType(resource, close).getThrownTypes()) &&
env.info.lint.isEnabled(LintCategory.TRY)) {
log.warning(pos, Warnings.TryResourceThrowsInterruptedExc(resource));
chk.isHandled(syms.interruptedExceptionType, types.memberType(resource, close).getThrownTypes())) {
env.info.lint.logIfEnabled(pos, LintWarnings.TryResourceThrowsInterruptedExc(resource));
}
}
}
Expand Down Expand Up @@ -4441,9 +4441,8 @@ public void visitSelect(JCFieldAccess tree) {
((VarSymbol)sitesym).isResourceVariable() &&
sym.kind == MTH &&
sym.name.equals(names.close) &&
sym.overrides(syms.autoCloseableClose, sitesym.type.tsym, types, true) &&
env.info.lint.isEnabled(LintCategory.TRY)) {
log.warning(tree, Warnings.TryExplicitCloseCall);
sym.overrides(syms.autoCloseableClose, sitesym.type.tsym, types, true)) {
env.info.lint.logIfEnabled(tree, LintWarnings.TryExplicitCloseCall);
}

// Disallow selecting a type from an expression
Expand All @@ -4470,9 +4469,9 @@ public void visitSelect(JCFieldAccess tree) {
// If the qualified item is not a type and the selected item is static, report
// a warning. Make allowance for the class of an array type e.g. Object[].class)
if (!sym.owner.isAnonymous()) {
chk.warnStatic(tree, Warnings.StaticNotQualifiedByType(sym.kind.kindName(), sym.owner));
env.info.lint.logIfEnabled(tree, LintWarnings.StaticNotQualifiedByType(sym.kind.kindName(), sym.owner));
} else {
chk.warnStatic(tree, Warnings.StaticNotQualifiedByType2(sym.kind.kindName()));
env.info.lint.logIfEnabled(tree, LintWarnings.StaticNotQualifiedByType2(sym.kind.kindName()));
}
}

Expand Down Expand Up @@ -4685,7 +4684,7 @@ else if (ownOuter.hasTag(CLASS) && site != ownOuter) {
if (s != null &&
s.isRaw() &&
!types.isSameType(v.type, v.erasure(types))) {
chk.warnUnchecked(tree.pos(), Warnings.UncheckedAssignToVar(v, s));
chk.warnUnchecked(tree.pos(), LintWarnings.UncheckedAssignToVar(v, s));
}
}
// The computed type of a variable is the type of the
Expand Down Expand Up @@ -4883,7 +4882,7 @@ public Type checkMethod(Type site,
if (s != null && s.isRaw() &&
!types.isSameTypes(sym.type.getParameterTypes(),
sym.erasure(types).getParameterTypes())) {
chk.warnUnchecked(env.tree.pos(), Warnings.UncheckedCallMbrOfRawType(sym, s));
chk.warnUnchecked(env.tree.pos(), LintWarnings.UncheckedCallMbrOfRawType(sym, s));
}
}

Expand Down Expand Up @@ -4933,7 +4932,7 @@ public Type checkMethod(Type site,
argtypes = argtypes.map(checkDeferredMap);

if (noteWarner.hasNonSilentLint(LintCategory.UNCHECKED)) {
chk.warnUnchecked(env.tree.pos(), Warnings.UncheckedMethInvocationApplied(kindName(sym),
chk.warnUnchecked(env.tree.pos(), LintWarnings.UncheckedMethInvocationApplied(kindName(sym),
sym.name,
rs.methodArguments(sym.type.getParameterTypes()),
rs.methodArguments(argtypes.map(checkDeferredMap)),
Expand Down
Loading

0 comments on commit c49f7a5

Please sign in to comment.