Fix multiple time imports #659
Conversation
| @@ -357,6 +418,18 @@ package imp_type | |||
|
|
|||
| type DotImportedType string | |||
|
|
|||
|
|
|||
|
|
|||
There was a problem hiding this comment.
please avoid this many empty lines; one at a time should be enough.
| ) | ||
|
|
||
| func regularAndUnusedDotImport() { | ||
| const localConst = imp_mult.MultImpStr |
There was a problem hiding this comment.
is this how you're keeping one of the imports used? it's a bit fragile, because we might end up "inlining" or simplifying all constant declarations in the future as part of obfuscation. it's probably better to have a real use via a func call.
There was a problem hiding this comment.
const is specially chosen because if i pass it as a parameter, then literals will just inline it and obfuscate it :)
There was a problem hiding this comment.
what I mean is to have each of these imported packages declare a dummy func, and the importer package imports the func to have a "used" import that remains used even with -literals.
| if !ok { | ||
| return true | ||
| } | ||
|
|
||
| if pkg := uses.Pkg(); pkg != nil { | ||
| usedImports[pkg.Path()] = true | ||
| // Handle qualified identifiers, cover renamed and simple imports |
There was a problem hiding this comment.
| // Handle qualified identifiers, cover renamed and simple imports | |
| // An identifier that directly references an imported package; cannot be a dot-import. | |
| // For example, this is the identifier `pkgfoo` in the code `var _ = pkgfoo.Bar`. |
| func multipleCheck(vals ...string) { | ||
| if len(vals) < 2 { | ||
| return | ||
| } | ||
| for i := 0; i < len(vals)-1; i++ { | ||
| if vals[i] != vals[i+1] { | ||
| panic("vals must be equals") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
not sure if this is necessary; if constant values were broken, our existing tests would already catch that :)
| // Handle dot-imported declarations | ||
| // If identifier is right part of SelectorExpr, it means it is a regular or renamed import |
There was a problem hiding this comment.
| // Handle dot-imported declarations | |
| // If identifier is right part of SelectorExpr, it means it is a regular or renamed import | |
| // Only dot-imported packages remain to be tracked. | |
| // If the identifier is the right part of a selector expression, like `bar` in `var _ = foo.bar`, | |
| // then it cannot be an identifier from a dot-imported package. |
| return true | ||
| } | ||
|
|
||
| // If package is missing or equal to current one it is not a dot-import case |
There was a problem hiding this comment.
| // If package is missing or equal to current one it is not a dot-import case | |
| // An identifier from a dot-imported package belongs to a dependency package. |
| return true | ||
| } | ||
|
|
||
| // Verify that identifier is declared in another package |
There was a problem hiding this comment.
| // Verify that identifier is declared in another package | |
| // Finally, verify that the identifier is defined in its package scope. |
Why is this last check needed, by the way?
|
We merged the other PR, and I agree the other approach is better :) Closing. |
Marking imports as used must consider its name and alias for correct processing of multiple time import
Fix: #658