Skip to content

Commit

Permalink
Merge pull request #10612 from som-snytt/lint/booleans
Browse files Browse the repository at this point in the history
Lint unnamed boolean literal args [ci: last-only]
  • Loading branch information
lrytz committed Dec 11, 2023
2 parents 233a548 + 2b7e0fe commit d561c4e
Show file tree
Hide file tree
Showing 108 changed files with 490 additions and 346 deletions.
4 changes: 2 additions & 2 deletions src/compiler/scala/reflect/macros/contexts/Typers.scala
Expand Up @@ -53,13 +53,13 @@ trait Typers {

def inferImplicitValue(pt: Type, silent: Boolean = true, withMacrosDisabled: Boolean = false, pos: Position = enclosingPosition): Tree = {
macroLogVerbose(s"inferring implicit value of type $pt, macros = ${!withMacrosDisabled}")
universe.analyzer.inferImplicit(universe.EmptyTree, pt, false, callsiteTyper.context, silent, withMacrosDisabled, pos, (pos, msg) => throw TypecheckException(pos, msg))
universe.analyzer.inferImplicit(universe.EmptyTree, pt, isView = false, callsiteTyper.context, silent, withMacrosDisabled, pos, (pos, msg) => throw TypecheckException(pos, msg))
}

def inferImplicitView(tree: Tree, from: Type, to: Type, silent: Boolean = true, withMacrosDisabled: Boolean = false, pos: Position = enclosingPosition): Tree = {
macroLogVerbose(s"inferring implicit view from $from to $to for $tree, macros = ${!withMacrosDisabled}")
val viewTpe = universe.appliedType(universe.definitions.FunctionClass(1).toTypeConstructor, List(from, to))
universe.analyzer.inferImplicit(tree, viewTpe, true, callsiteTyper.context, silent, withMacrosDisabled, pos, (pos, msg) => throw TypecheckException(pos, msg))
universe.analyzer.inferImplicit(tree, viewTpe, isView = true, callsiteTyper.context, silent, withMacrosDisabled, pos, (pos, msg) => throw TypecheckException(pos, msg))
}

def resetLocalAttrs(tree: Tree): Tree = universe.resetAttrs(universe.duplicateAndKeepPositions(tree))
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/NewLinePrintWriter.scala
Expand Up @@ -15,7 +15,7 @@ import java.io.{Writer, PrintWriter}

class NewLinePrintWriter(out: Writer, autoFlush: Boolean)
extends PrintWriter(out, autoFlush) {
def this(out: Writer) = this(out, false)
def this(out: Writer) = this(out, autoFlush = false)
override def println(): Unit = { print("\n"); flush() }
}

4 changes: 2 additions & 2 deletions src/compiler/scala/tools/nsc/PhaseAssembly.scala
Expand Up @@ -62,8 +62,8 @@ trait PhaseAssembly {
node
}

def softConnectNodes(frm: Node, to: Node) = connectNodes(Edge(frm, to, false))
def hardConnectNodes(frm: Node, to: Node) = connectNodes(Edge(frm, to, true))
def softConnectNodes(frm: Node, to: Node) = connectNodes(Edge(frm, to, hard = false))
def hardConnectNodes(frm: Node, to: Node) = connectNodes(Edge(frm, to, hard = true))

// Connect the frm and to nodes in the edge and add it to the set of edges.
private def connectNodes(e: Edge): Unit = {
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/scala/tools/nsc/Reporting.scala
Expand Up @@ -628,7 +628,8 @@ object Reporting {
LintPerformance,
LintIntDivToFloat,
LintUniversalMethods,
LintNumericMethods
LintNumericMethods,
LintNamedBooleans
= lint()

sealed class Feature extends WarningCategory {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/ast/Printers.scala
Expand Up @@ -195,7 +195,7 @@ trait Printers extends scala.reflect.internal.Printers { this: Global =>

def asString(t: Tree): String = render(t, newStandardTreePrinter, settings.printtypes, settings.uniqid, settings.Yshowsymowners, settings.Yshowsymkinds)
def asCompactString(t: Tree): String = render(t, newCompactTreePrinter, settings.printtypes, settings.uniqid, settings.Yshowsymowners, settings.Yshowsymkinds)
def asCompactDebugString(t: Tree): String = render(t, newCompactTreePrinter, true, true, true, true)
def asCompactDebugString(t: Tree): String = render(t, newCompactTreePrinter, printTypes = true, printIds = true, printOwners = true, printKinds = true)

def newStandardTreePrinter(writer: PrintWriter): AstTreePrinter = new AstTreePrinter(writer)
def newCompactTreePrinter(writer: PrintWriter): CompactTreePrinter = new CompactTreePrinter(writer)
Expand Down
11 changes: 8 additions & 3 deletions src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
Expand Up @@ -1987,9 +1987,14 @@ self =>
* }}}
*/
def argumentExprs(): List[Tree] = {
def args(): List[Tree] = commaSeparated(
if (isIdent) treeInfo.assignmentToMaybeNamedArg(expr()) else expr()
)
def args(): List[Tree] = commaSeparated {
val checkNamedArg = isIdent
expr() match {
case t @ Assign(id: Ident, rhs) if checkNamedArg => atPos(t.pos)(NamedArg(id, rhs))
case t @ Literal(Constant(_: Boolean)) => t.updateAttachment(UnnamedArg)
case t => t
}
}
in.token match {
case LBRACE => List(blockExpr())
case LPAREN => inParens {
Expand Down
Expand Up @@ -349,7 +349,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
val (staticArgs, dynamicArgs) = staticAndDynamicArgs.splitAt(staticAndDynamicArgs.length - numDynamicArgs)
val bootstrapDescriptor = staticHandleFromSymbol(bootstrapMethodRef)
val bootstrapArgs = staticArgs.map({case t @ Literal(c: Constant) => bootstrapMethodArg(c, t.pos) case x => throw new MatchError(x)})
val descriptor = methodBTypeFromMethodType(qual.symbol.info, false)
val descriptor = methodBTypeFromMethodType(qual.symbol.info, isConstructor=false)
genLoadArguments(dynamicArgs, qual.symbol.info.params.map(param => typeToBType(param.info)))
mnode.visitInvokeDynamicInsn(qual.symbol.name.encoded, descriptor.descriptor, bootstrapDescriptor, bootstrapArgs : _*)

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala
Expand Up @@ -1151,7 +1151,7 @@ object BTypes {
}
}

val EmptyInlineInfo = InlineInfo(false, None, SortedMap.empty, None)
val EmptyInlineInfo = InlineInfo(isEffectivelyFinal = false, sam = None, methodInfos = SortedMap.empty, warning = None)

/**
* Metadata about a method, used by the inliner.
Expand Down
Expand Up @@ -56,7 +56,7 @@ abstract class ByteCodeRepository extends PerRunInit {
* Note - although this is typed a mutable.Map, individual simple get and put operations are threadsafe as the
* underlying data structure is synchronized.
*/
val parsedClasses: mutable.Map[InternalName, Either[ClassNotFound, ClassNode]] = recordPerRunCache(LruMap[InternalName, Either[ClassNotFound, ClassNode]](maxCacheSize, true))
val parsedClasses: mutable.Map[InternalName, Either[ClassNotFound, ClassNode]] = recordPerRunCache(LruMap[InternalName, Either[ClassNotFound, ClassNode]](maxCacheSize, threadsafe = true))

/**
* Contains the internal names of all classes that are defined in Java source files of the current
Expand Down
16 changes: 10 additions & 6 deletions src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala
Expand Up @@ -298,10 +298,14 @@ abstract class CallGraph {
/**
* Just a named tuple used as return type of `analyzeCallsite`.
*/
private case class CallsiteInfo(isStaticallyResolved: Boolean, sourceFilePath: Option[String],
annotatedInline: Boolean, annotatedNoInline: Boolean,
samParamTypes: IntMap[ClassBType],
warning: Option[CalleeInfoWarning])
private case class CallsiteInfo(
isStaticallyResolved: Boolean = false,
sourceFilePath: Option[String] = None,
annotatedInline: Boolean = false,
annotatedNoInline: Boolean = false,
samParamTypes: IntMap[ClassBType] = IntMap.empty,
warning: Option[CalleeInfoWarning],
)

/**
* Analyze a callsite and gather meta-data that can be used for inlining decisions.
Expand Down Expand Up @@ -357,12 +361,12 @@ abstract class CallGraph {

case None =>
val warning = MethodInlineInfoMissing(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, calleeDeclarationClassBType.info.orThrow.inlineInfo.warning)
CallsiteInfo(false, None, false, false, IntMap.empty, Some(warning))
CallsiteInfo(warning = Some(warning))
}
} catch {
case Invalid(noInfo: NoClassBTypeInfo) =>
val warning = MethodInlineInfoError(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, noInfo)
CallsiteInfo(false, None, false, false, IntMap.empty, Some(warning))
CallsiteInfo(warning = Some(warning))
}
}

Expand Down
Expand Up @@ -166,4 +166,4 @@ object InlineInfoAttribute {
* In order to instruct the ASM framework to deserialize the ScalaInlineInfo attribute, we need
* to pass a prototype instance when running the class reader.
*/
object InlineInfoAttributePrototype extends InlineInfoAttribute(InlineInfo(false, null, null, null))
object InlineInfoAttributePrototype extends InlineInfoAttribute(InlineInfo(isEffectivelyFinal = false, sam = null, methodInfos = null, warning = null))
4 changes: 2 additions & 2 deletions src/compiler/scala/tools/nsc/backend/jvm/opt/LruMap.scala
Expand Up @@ -17,8 +17,8 @@ import scala.jdk.CollectionConverters._
import java.util.{LinkedHashMap, Collections, Map => JMap}

object LruMap{
def apply[K,V](maxSize:Int, threadsafe:Boolean): Map[K,V] = {
require (maxSize > 0)
def apply[K,V](maxSize: Int, threadsafe: Boolean): Map[K,V] = {
require(maxSize > 0)
val basic = new LruMapImpl[K,V](maxSize)
val threaded = if (threadsafe) Collections.synchronizedMap(basic) else basic

Expand Down
17 changes: 6 additions & 11 deletions src/compiler/scala/tools/nsc/classpath/ClassPathFactory.scala
Expand Up @@ -15,7 +15,7 @@ package scala.tools.nsc.classpath
import scala.reflect.io.{AbstractFile, VirtualDirectory}
import scala.tools.nsc.{CloseableRegistry, Settings}
import FileUtils.AbstractFileOps
import scala.tools.nsc.util.ClassPath
import scala.tools.nsc.util.ClassPath, ClassPath.{expandDir, expandPath}

/**
* Provides factory methods for classpath. When creating classpath instances for a given path,
Expand All @@ -37,11 +37,6 @@ class ClassPathFactory(settings: Settings, closeableRegistry: CloseableRegistry
dir <- Option(settings.pathFactory.getDirectory(file))
} yield createSourcePath(dir)


def expandPath(path: String, expandStar: Boolean = true): List[String] = scala.tools.nsc.util.ClassPath.expandPath(path, expandStar)

def expandDir(extdir: String): List[String] = scala.tools.nsc.util.ClassPath.expandDir(extdir)

def contentsOfDirsInPath(path: String): List[ClassPath] =
for {
dir <- expandPath(path, expandStar = false)
Expand All @@ -50,18 +45,18 @@ class ClassPathFactory(settings: Settings, closeableRegistry: CloseableRegistry
} yield newClassPath(entry)

def classesInExpandedPath(path: String): IndexedSeq[ClassPath] =
classesInPathImpl(path, expand = true).toIndexedSeq
classesInPathImpl(path, expandStar = true).toIndexedSeq

def classesInPath(path: String) = classesInPathImpl(path, expand = false)
def classesInPath(path: String): List[ClassPath] = classesInPathImpl(path, expandStar = false)

def classesInManifest(useManifestClassPath: Boolean) =
def classesInManifest(useManifestClassPath: Boolean): List[ClassPath] =
if (useManifestClassPath) scala.tools.nsc.util.ClassPath.manifests.map(url => newClassPath(AbstractFile getResources url))
else Nil

// Internal
protected def classesInPathImpl(path: String, expand: Boolean) =
protected def classesInPathImpl(path: String, expandStar: Boolean): List[ClassPath] =
for {
file <- expandPath(path, expand)
file <- expandPath(path, expandStar)
dir <- {
def asImage = if (file.endsWith(".jimage")) Some(settings.pathFactory.getFile(file)) else None
Option(settings.pathFactory.getDirectory(file)).orElse(asImage)
Expand Down
Expand Up @@ -150,7 +150,7 @@ object JrtClassPath {
val ctSym = Paths.get(javaHome).resolve("lib").resolve("ct.sym")
if (Files.notExists(ctSym)) None
else {
val classPath = ctSymClassPathCache.getOrCreate(v, ctSym :: Nil, () => new CtSymClassPath(ctSym, v.toInt), closeableRegistry, true)
val classPath = ctSymClassPathCache.getOrCreate(v, ctSym :: Nil, () => new CtSymClassPath(ctSym, v.toInt), closeableRegistry, checkStamps = true)
Some(classPath)
}
} catch {
Expand All @@ -159,7 +159,7 @@ object JrtClassPath {
case _ =>
try {
val fs = FileSystems.getFileSystem(URI.create("jrt:/"))
val classPath = jrtClassPathCache.getOrCreate((), Nil, () => new JrtClassPath(fs), closeableRegistry, false)
val classPath = jrtClassPathCache.getOrCreate((), Nil, () => new JrtClassPath(fs), closeableRegistry, checkStamps = false)
Some(classPath)
} catch {
case _: ProviderNotFoundException | _: FileSystemNotFoundException => None
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/javac/JavaScanners.scala
Expand Up @@ -1026,7 +1026,7 @@ trait JavaScanners extends ast.parser.ScannersCommon {
}

class JavaUnitScanner(unit: CompilationUnit) extends JavaScanner {
in = new JavaCharArrayReader(new ArraySeq.ofChar(unit.source.content), true, syntaxError)
in = new JavaCharArrayReader(new ArraySeq.ofChar(unit.source.content), decodeUni = true, syntaxError)
init()
def error(pos: Int, msg: String) = reporter.error(pos, msg)
def incompleteInputError(pos: Int, msg: String) = currentRun.parsing.incompleteInputError(pos, msg)
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/plugins/Plugins.scala
Expand Up @@ -182,7 +182,7 @@ trait Plugins { global: Global =>
def findMacroClassLoader(): ClassLoader = {
val classpath: Seq[URL] = if (settings.YmacroClasspath.isSetByUser) {
for {
file <- ClassPath.expandPath(settings.YmacroClasspath.value, true)
file <- ClassPath.expandPath(settings.YmacroClasspath.value, expandStar = true)
af <- Option(settings.pathFactory.getDirectory(file))
} yield af.file.toURI.toURL
} else global.classPath.asURLs
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
Expand Up @@ -241,7 +241,7 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
val Ydumpclasses = StringSetting ("-Ydump-classes", "dir", "Dump the generated bytecode to .class files (useful for reflective compilation that utilizes in-memory classloaders).", "")
val stopAfter = PhasesSetting ("-Ystop-after", "Stop after") withAbbreviation ("-stop") // backward compat
val stopBefore = PhasesSetting ("-Ystop-before", "Stop before")
val Yrangepos = BooleanSetting ("-Yrangepos", "Use range positions for syntax trees.", true)
val Yrangepos = BooleanSetting ("-Yrangepos", "Use range positions for syntax trees.", default = true)
val Yvalidatepos = PhasesSetting ("-Yvalidate-pos", s"Validate positions after the given phases (implies ${Yrangepos.name})") withPostSetHook (_ => Yrangepos.value = true)
val Yreifycopypaste = BooleanSetting ("-Yreify-copypaste", "Dump the reified trees in copypasteable representation.")
val Ymacroexpand = ChoiceSetting ("-Ymacro-expand", "policy", "Control expansion of macros, useful for scaladoc and presentation compiler.", List(MacroExpand.Normal, MacroExpand.None, MacroExpand.Discard), MacroExpand.Normal)
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/scala/tools/nsc/settings/Warnings.scala
Expand Up @@ -216,6 +216,7 @@ trait Warnings {
val NumericMethods = LintWarning("numeric-methods", "Dubious usages, such as `42.isNaN`.")
val ArgDiscard = LintWarning("arg-discard", "-Wvalue-discard for adapted arguments.")
val IntDivToFloat = LintWarning("int-div-to-float", "Warn when an integer division is converted (widened) to floating point: `(someInt / 2): Double`.")
val NamedBooleans = LintWarning("named-booleans", "Boolean literals should be named args unless param is @deprecatedName.")

def allLintWarnings = values.toSeq.asInstanceOf[Seq[LintWarning]]
}
Expand Down Expand Up @@ -252,6 +253,7 @@ trait Warnings {
def lintNumericMethods = lint.contains(NumericMethods)
def lintArgDiscard = lint.contains(ArgDiscard)
def lintIntDivToFloat = lint.contains(IntDivToFloat)
def lintNamedBooleans = lint.contains(NamedBooleans)

// The Xlint warning group.
val lint = MultiChoiceSetting(
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/tasty/TreeUnpickler.scala
Expand Up @@ -689,7 +689,7 @@ class TreeUnpickler[Tasty <: TastyUniverse](
case ANNOTATION =>
annotFns = readAnnot(ctx) :: annotFns
case tag =>
assert(assertion = false, s"illegal modifier tag ${astTagToString(tag)} at $currentAddr, end = $end")
assert(false, s"illegal modifier tag ${astTagToString(tag)} at $currentAddr, end = $end")
}
}
(flags, if (ctx.ignoreAnnotations) Nil else annotFns.reverse, privateWithin)
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/tasty/bridge/ContextOps.scala
Expand Up @@ -261,7 +261,7 @@ trait ContextOps { self: TastyUniverse =>
case TastyName.Root | TastyName.RootPkg => loadingMirror.RootPackage
case TastyName.EmptyPkg => loadingMirror.EmptyPackage
case fullname =>
symOrDependencyError(false, true, fullname)(loadingMirror.getPackage(encodeTermName(fullname).toString))
symOrDependencyError(isObject = false, isPackage = true, fullname)(loadingMirror.getPackage(encodeTermName(fullname).toString))
}

private def symOrDependencyError(isObject: Boolean, isPackage: Boolean, fullname: TastyName)(sym: => Symbol): Symbol = {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/transform/Constructors.scala
Expand Up @@ -366,7 +366,7 @@ abstract class Constructors extends Statics with Transform with TypingTransforme
def rewriteUnspecialized(assignee: Symbol, stat: Tree): Tree = {
assert(ctorParams(genericClazz).length == primaryConstrParams.length, "Bad param len")
// this is just to make private fields public
(new specializeTypes.ImplementationAdapter(ctorParams(genericClazz), primaryConstrParams, null, true))(stat)
(new specializeTypes.ImplementationAdapter(ctorParams(genericClazz), primaryConstrParams, null, addressFields = true))(stat)
// also make assigned fields mutable so they don't end up final in bytecode
// and mark the specialized class constructor for a release fence addition
if (assignee.isField)
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/scala/tools/nsc/transform/Erasure.scala
Expand Up @@ -52,11 +52,11 @@ abstract class Erasure extends InfoTransform
}

private object NeedsSigCollector {
private val NeedsSigCollector_true = new NeedsSigCollector(true)
private val NeedsSigCollector_false = new NeedsSigCollector(false)
private val NeedsSigCollector_true = new NeedsSigCollector(isClassConstructor = true)
private val NeedsSigCollector_false = new NeedsSigCollector(isClassConstructor = false)
def apply(isClassConstructor: Boolean) = if (isClassConstructor) NeedsSigCollector_true else NeedsSigCollector_false
}
private class NeedsSigCollector(isClassConstructor: Boolean) extends TypeCollector(false) {
private class NeedsSigCollector(isClassConstructor: Boolean) extends TypeCollector(initial = false) {
def apply(tp: Type): Unit =
if (!result) {
tp match {
Expand Down

0 comments on commit d561c4e

Please sign in to comment.