Skip to content
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

Add Type.Addr().Implements() in DSL #137

Closed
wants to merge 18 commits into from

Conversation

sebastien-rosset
Copy link
Contributor

@sebastien-rosset sebastien-rosset commented Dec 15, 2020

Fix for #129

Suppose in a particular application the developer must instantiate a Job struct using a factory method instead of plain struct initializer. The factory method initializes the fields as needed. If in the future new fields are added, the factory method will take care of doing the initialization in one place.

I want to print a warning such as Replace struct initialization with call to factory method NewXyz().

Suppose further there are many generated structs (not just one one Job). All these structs must be instantiated using a factory method. It just so happens that all these structs implements a particular interface.

Consider the following code:

package main

import (
	"worker"
)

func main() {
	j := worker.Job{Name: "test"} // Instead we want the developer to invoke a factory method.
	j.Run()
}

If there is a single Job struct, then it's easy to create a match rule using Match("worker.Job{$*_}"). But what if there are hundreds a structs, possibly code-generated, that implements a specific interface?
In that case, we can start by using the existing Implements keyword in the DSL:

	m.Match(`$x{$*_}`).
		Where(m["x"].Type.Implements("worker.Runner")).
		Report("Replace struct initialization with call to factory method")

However, this does not work because as shown in the code sample, worker.Job is not a pointer, therefore Implements("worker.Runner") would always return false.

The proposal is to add a new fluent.ExprType.Addr() ExprType function in the DSL. The function returns a type pointer, therefore any ExprType function (including Implements) can be called on the returned ExprType value.

The new Addr() method can be used for any use case where the matching code is not a pointer, but a pointer type is needed to invoke Implements.

func invalidObjectInitialization(m fluent.Matcher) {
	m.Import(`.../worker`)

	m.Match(`$x{$*_}`).
		Where(m["x"].Type.Addr().Implements("worker.Runner")).
		Report("Replace struct initialization with call to factory method")
}

I don't have good use cases for Type.Addr().Is(), Type.Addr().IsConvertible and Type.Addr().IsAssignable() so I didn't add these. Seeking feedback before deciding what to do next.

@sebastien-rosset sebastien-rosset changed the title Add Pointer() function in DSL Add Type.Pointer().Implements() in DSL Dec 16, 2020
@sebastien-rosset
Copy link
Contributor Author

@cristaloleg , @quasilyte , any thoughts about this change?

@sebastien-rosset sebastien-rosset changed the title Add Type.Pointer().Implements() in DSL Add Type.Addr().Implements() in DSL Dec 30, 2020
@sebastien-rosset
Copy link
Contributor Author

Closing. Fixed by #171 and #173

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant