-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add warnings for inferred flexible types in public methods and fields #23880
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
Changes from 3 commits
72c545e
23b4a2d
c46c9bf
1d3d0c4
77dae85
f879e64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3089,13 +3089,32 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |
//todo: make sure dependent method types do not depend on implicits or by-name params | ||
} | ||
|
||
/** (1) Check that the signature of the class member does not return a repeated parameter type | ||
* (2) Make sure the definition's symbol is `sym`. | ||
* (3) Set the `defTree` of `sym` to be `mdef`. | ||
/** (1) Check that the signature of the class member does not return a repeated parameter type. | ||
* (2) Check that the signature of the public class member does not expose a flexible type. | ||
* (3) Make sure the definition's symbol is `sym`. | ||
* (4) Set the `defTree` of `sym` to be `mdef`. | ||
*/ | ||
private def postProcessInfo(mdef: MemberDef, sym: Symbol)(using Context): MemberDef = | ||
if (!sym.isOneOf(Synthetic | InlineProxy | Param) && sym.info.finalResultType.isRepeatedParam) | ||
report.error(em"Cannot return repeated parameter type ${sym.info.finalResultType}", sym.srcPos) | ||
|
||
// Warn if a public method/field exposes FlexibleType in its result type under explicit nulls | ||
// and encourage explicit annotation. | ||
if ctx.phase.isTyper && ctx.explicitNulls && !ctx.isJava | ||
&& sym.exists && sym.isPublic && sym.owner.isClass | ||
noti0na1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
&& !sym.isOneOf(Synthetic | InlineProxy | Param) then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] The complex conditional logic spans multiple lines without clear grouping. Consider extracting this logic into a helper method like Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
val resTp = sym.info.finalResultType | ||
noti0na1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if resTp.existsPart(_.isInstanceOf[FlexibleType], StopAt.Static) then | ||
val suggestion = resTp match | ||
case ft: FlexibleType => | ||
val hi = ft.hi | ||
i"Consider annotating the type as ${hi} or ${hi} | Null explicitly" | ||
sjrd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
case _ => "Consider annotating the type explicitly" | ||
report.warning( | ||
em"Public ${sym.show} exposes a flexible type ${resTp} in its inferred signature. $suggestion", | ||
noti0na1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sym.srcPos | ||
) | ||
|
||
mdef.ensureHasSym(sym) | ||
mdef.setDefTree | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -64,20 +64,21 @@ class ScriptEngine extends AbstractScriptEngine { | |||
|
||||
object ScriptEngine { | ||||
import java.util.Arrays | ||||
import java.util.List | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The import for
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||
import scala.util.Properties | ||||
|
||||
class Factory extends ScriptEngineFactory { | ||||
def getEngineName = "Scala REPL" | ||||
def getEngineVersion = "3.0" | ||||
def getExtensions = Arrays.asList("scala") | ||||
def getExtensions: List[String] = Arrays.asList("scala") | ||||
def getLanguageName = "Scala" | ||||
def getLanguageVersion = Properties.versionString | ||||
def getMimeTypes = Arrays.asList("application/x-scala") | ||||
def getNames = Arrays.asList("scala") | ||||
def getMimeTypes: List[String] = Arrays.asList("application/x-scala") | ||||
def getNames: List[String] = Arrays.asList("scala") | ||||
|
||||
def getMethodCallSyntax(obj: String, m: String, args: String*) = s"$obj.$m(${args.mkString(", ")})" | ||||
def getMethodCallSyntax(obj: String, m: String, args: String*): String = s"$obj.$m(${args.mkString(", ")})" | ||||
|
||||
def getOutputStatement(toDisplay: String) = s"""print("$toDisplay")""" | ||||
def getOutputStatement(toDisplay: String): String = s"""print("$toDisplay")""" | ||||
|
||||
def getParameter(key: String): Object = key match { | ||||
case JScriptEngine.ENGINE => getEngineName | ||||
|
@@ -88,7 +89,7 @@ object ScriptEngine { | |||
case _ => null | ||||
} | ||||
|
||||
def getProgram(statements: String*) = statements.mkString("; ") | ||||
def getProgram(statements: String*): String = statements.mkString("; ") | ||||
|
||||
def getScriptEngine: JScriptEngine = new ScriptEngine | ||||
} | ||||
|
Uh oh!
There was an error while loading. Please reload this page.