-
Notifications
You must be signed in to change notification settings - Fork 11
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
[DONOTMERGE] Upgrade Disable rule to scalafix v1 #23
base: master
Are you sure you want to change the base?
Conversation
.map(Disable(index, _)) | ||
override def withConfiguration(config: Configuration): Configured[Rule] = { | ||
// should this use DisableConfig.default? | ||
config.conf.getOrElse("disable", "Disable")(this.config).map { newConfig => Disable(newConfig) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this bit
// errorCategory | ||
// .copy(id = id) | ||
// .at(message, pos) | ||
DisableDiagnostic(symbol, details = details, position = pos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to be cleaned up
isBlocked.findMatch(s).map { disabled => | ||
SymbolOps.normalize(s) match { | ||
case g: Symbol if g.info.get.signature.toString() != "<init>" => | ||
Left(Patch.lint(DisableDiagnostic(s, "", t.pos)).atomic) // XXX this is incorrect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix this
} | ||
|
||
private def checkSynthetics(ctx: RuleCtx): Seq[Diagnostic] = { | ||
// XXX what goes here? | ||
private def checkSynthetics(implicit doc: SemanticDocument): Seq[Patch] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checkSynthetics I don't know what the v1 equivalent is
@@ -33,7 +32,7 @@ case class DisabledSymbol( | |||
def matches(symbol: Symbol): Boolean = { | |||
this.symbol match { | |||
case Some(s) => | |||
SymbolOps.isSameNormalized(symbol, s) | |||
symbol.normalized.equals(s.normalized) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the same
@@ -53,10 +52,14 @@ object DisabledSymbol { | |||
} else { | |||
msg | |||
} | |||
|
|||
implicit lazy val symbolV1Reader: ConfDecoder[Symbol] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
best guess at what Symbol.Global equivalent is
DO NOT MERGE, STILL BEING WORKED ON
This is a PR to upgrade the Disable scalafix rule from scalafix v0 to scalafix v1.