johnno1962 / Refactorator

Xcode Plugin that Refactors Swift & Objective-C
MIT License
991 stars 35 forks source link

refactor struct properties search incomplete #3

Closed owenzhao closed 8 years ago

owenzhao commented 8 years ago

I have a struct in file person.swift, say

struct Person {
    var name:String = ""
}

and in another file AppDelegate.swift, I use the struct, say:

extension AppDelegate {
func foo() {
    var john = Person()
    john.name = 'John Smith"
}

When I refactor name in person.swift, I can only find the name in the file person.swift. which is incomplete.

When I refactor name in AppDelegate.swift, I can find name in both files.

johnno1962 commented 8 years ago

Is person.swift in a separate framework target?

owenzhao commented 8 years ago

No. They both target to my app.

My code is not the same as the example. And I find there are two ids. for the same property.

s:vV15ASSFontReplacer17ASSFontComponents5diffsGVSs10DictionaryOS15ASSFontPropertySS s:FV15ASSFontReplacer17ASSFontComponentscFMS0_FT5diffsGVSs10DictionaryOS_15ASSFontPropertySS_S0

owenzhao commented 8 years ago

In the foo.swift, the code is

struct ASSFontComponents {
    var diffs:[ASSFontProperty:String]

    var output:String {
        let left = "{"
        let right = "}"
        let prefix = "\\"
        let divider = ","

        let results = self.diffs.map { (key, value) -> String in
            return prefix + key.rawValue + value
            }.joinWithSeparator(divider)

        return left + results + right
    }

    var dictionary:[String:String] {
        var results = [String:String]()

        for (property, value) in self.diffs {
            results.updateValue(value, forKey: property.rawValue)
        }

        return results
    }

    init(diffs:[ASSFontProperty:String]) {
        self.diffs = diffs
    }

    init(dictionary:[String:String]) {
        var diffs = [ASSFontProperty:String]()

        for (rawValue, value) in dictionary {
            diffs.updateValue(value, forKey: ASSFontProperty(rawValue: rawValue)!)
        }

        self.init(diffs:diffs)
    }
}

If I refactor diffs in this file, it can find

ASSFontCore.swift:108 var diffs:[ASSFontProperty:String] ASSFontCore.swift:116 let results = self.diffs.map { (key, value) -> String in ASSFontCore.swift:126 for (property, value) in self.diffs { ASSFontCore.swift:134 self.diffs = diffs

There are 10 diffs in total. It finds 5 of them.

johnno1962 commented 8 years ago

In this case it is performing correctly surely. There are only 5 references to the property diffs in struct ASSFontComponents. The others are arguments or local variables.

owenzhao commented 8 years ago

True. Some of the diffs are local variables. But the diffs in init() should be included, in my option.

johnno1962 commented 8 years ago

I would agree but SourceKit does not pick these up as being the same entity.

owenzhao commented 8 years ago

Thanks. Now I know why in the other file the diffs can't find here. Because they are the diffs in init(diffs:), which is not the same diffs as struct property according to SourceKit.

Now the random behavior shows its regularity to me. Thanks again.

johnno1962 commented 8 years ago

Close the issue? I can’t explain why SourceKit gives two USR ids for the same entity depending on what file you start in unless they are in different targets.

owenzhao commented 8 years ago

Still exists an issue. When I refractor the init(diffs:), I can find 0 elements, but if I refactor the diffs in the other file, I can find all in the other file, as well as those in the init(diffs:). It seams that the init(diffs:) has two ids?

s:vFV15ASSFontReplacer17ASSFontComponentscFMS0_FT5diffsGVSs10DictionaryOS_15ASSFontPropertySS__S0_L_5diffsGS1_S2SS s:FV15ASSFontReplacer17ASSFontComponentscFMS0_FT5diffsGVSs10DictionaryOS_15ASSFontPropertySS_S0

The prior is in the init(diffs:), the other is in the other file.

owenzhao commented 8 years ago

In the other file, the code is this:

let sFontConvertRules:[GeneralFontType:[FontComponentsDictionary]] = [
            "楷":[ASSFontComponents(diffs:[.Fontname: "楷体-简"]).dictionary],
            "宋":[ASSFontComponents(diffs:[.Fontname: "宋体-简"]).dictionary],
            "黑":[ASSFontComponents(diffs:[.Fontname: "兰亭黑-简", .Bold: "0"]).dictionary],
            "圆":[ASSFontComponents(diffs:[.Fontname: "圆体-简"]).dictionary],
            "其它":[ASSFontComponents(diffs:[.Fontname: "兰亭黑-简"]).dictionary]
        ]
johnno1962 commented 8 years ago

What are you selecting? ASSFontComponents or the diffs argument?

owenzhao commented 8 years ago

Besides, the above code find the diffs in init(diffs:), but leave the diffs inside the init() away.

    init(diffs:[ASSFontProperty:String]) { // find this diffs
        self.diffs = diffs // left the last diffs
    }
owenzhao commented 8 years ago

"楷":[ASSFontComponents(diffs:[.Fontname: "楷体-简"]).dictionary],

I choose the diffs, and right click my mouse button.

johnno1962 commented 8 years ago

Sorry, but I think you’re probably pushing the limits of what SourceKit can do working with arguments and initialisers. diffs the argument is different from diffs the property though they should be related in a sense. Not much I can do.

owenzhao commented 8 years ago

I know the diffs in property is different from the diffs in init(), however, the diffs in init(diffs:) should be the same as the self.diffs = diffs.

I mean the last diffs, not the one begin with self.

johnno1962 commented 8 years ago

diffs the argument exposed externally must have a differnt USR than the argument/local variable in the initialiser function..

owenzhao commented 8 years ago

I make an example.

import Foundation

struct Person {
    var name:String

    init(aName:String) {
        self.name = aName
    }
}

func testOnly() {
    let john = Person(aName: "John")

    print(john)
}

When I refactor let john = Person(aName: "John")

It finds: testcode.swift:14 init(aName:String) { testcode.swift:20 let john = Person(aName: "John")

Indexing Complete. Symbol referenced in 2 places. usage

It should find self.name = aName as well.

johnno1962 commented 8 years ago

I agree but I don’t think sourcekit is making the association between the external argument and it’s usage as an local inside the function. If you make the external refactor you can pick up the missing refactor and fix it after when you try to build. Best the plugin can do the way things are

owenzhao commented 8 years ago

I know as the compiler shows the mistakes after refactoring. I have to refactor from external as I can't refactor anything if I choose from the either of the aNames in the init()

If I do that, it will show

Indexing Complete. Symbol referenced in 0 places. usage

owenzhao commented 8 years ago

I think if there is nothing you can do on this. You can close this issue now.

johnno1962 commented 8 years ago

OK, thanks for the report.