#554 Identifiers tab: for a unit found in several directories, the copy is picked by alphabetical directory order, not by search order

[Thomas Mueller]

The parser thread behind the Identifiers tab of the Uses Clause Manager (TUnitExportParserThread in GX_UnitExportsParser.pas) keeps its search directories in a sorted string list (FPaths.Sorted := True). Ota.GetAllPossiblePaths returns them roughly in the order the compiler searches them: the project's folder, the project's search path, the library path, then further folders such as those of the project's units, the project group and the VCL/RTL source. The sort throws that order away, and the .pas files are then collected directory by directory in alphabetical order.

The compiler takes the first copy of a unit it finds in search order. GExperts takes a different one when the same unit exists in more than one of these directories, for example:

• an RTL or VCL unit copied into the project folder to patch it, while the original is in the IDE's source folder;
• two versions of a library, or an old copy of a unit left behind in another folder on the path.

Consequences:

• Favourites mode (only the favourite units are parsed): each favourite is parsed from the copy in the alphabetically first directory. With C:\Program Files (x86)\Embarcadero\Studio\37.0\source\rtl\common\System.Classes.pas and a patched copy in D:\Projects\MyApp\, the original is parsed, although the compiler uses the patched copy. The tab then lists the identifiers of the wrong copy: an identifier that exists only in the copy the compiler uses is missing, one that exists only in the other copy is listed, and the file and line shown for an identifier are those of the wrong copy.
• Parse-all mode: every copy is parsed, so each identifier appears once per copy. The rows look the same (identifier and unit name) and differ only in the file shown in the status bar, so it is not visible which one the compiler would use.
• Declare Variable uses the same identifier list (TUsesClauseMgrExpert.TryGetIdentifiers). When it looks up a class's method (TUnitExportlist.FindExact), it tries the copies in alphabetical order of their full file names (TUnitExportlist.CompareItems) and takes the first that matches, so here too the copy is not the one the compiler uses.

Not affected: the browsing path rule added for bug #552 keeps the order of the browsing path, and it is only used for units of which no source is found in these directories at all.

Possible fix: keep FPaths in the order it is passed in, removing duplicates, and do the lookups that need a sorted list on a separate sorted copy. In favourites mode, take the first copy in that order. In parse-all mode, either parse only that copy or mark the others; for Declare Variable, the entries of one identifier could be kept in search order instead of file name order.

Found in a code review of the fix for bug #552. The consequences above come from reading the code; they have not been reproduced in the IDE.