Gives MojomFile a slice in order to store its imports in a specified order. We want to be able to store both the specified order of the imports and also a map from specified short name to canonical long name. Also makes clear the different roles of the two names and which is used where. R=azani@chromium.org, azani BUG=#461 Review URL: https://codereview.chromium.org/1423533003 .
diff --git a/mojom/mojom_parser/mojom/mojom_descriptor.go b/mojom/mojom_parser/mojom/mojom_descriptor.go index 03129f1..3384108 100644 --- a/mojom/mojom_parser/mojom/mojom_descriptor.go +++ b/mojom/mojom_parser/mojom/mojom_descriptor.go
@@ -21,10 +21,9 @@ // The associated MojomDescriptor Descriptor *MojomDescriptor - // The |FileName| is (derived from) the file name of the corresponding - // .mojom file. It is the unique identifier for this module within the - // |MojomFilesByName| field of |Descriptor| - FileName string + // The |CanonicalFileName| is the unique identifier for this module + // within the |MojomFilesByName| field of |Descriptor| + CanonicalFileName string // The module namespace is the identifier declared via the "module" // declaration in the .mojom file. @@ -33,16 +32,13 @@ // Attributes declared in the Mojom file at the module level. Attributes *Attributes - // The set of other MojomFiles imported by this one. The keys to the map - // are the file names as they appear in the importing .mojom file. The - // values are the canonicalized |FileName|s. The corresponding MojomFile may - // be obtained from the |MojomFilesByName| field of |Descriptor| using this - // canonicalized |FileName|. Note that when a .mojom file is first parsed - // only the keys of this map are populated because we don't know the - // canonicalized name of the imported file yet. It is only when the imported - // file is pre-processed in parser_driver.go that we discover the canonicalized - // name and add it to this map. - Imports map[string]string + // The set of other MojomFiles imported by this one. The corresponding + // MojomFile may be obtained from the |MojomFilesByName| field of + // |Descriptor| using the |CanonicalFileName| field of ImportedFile. + Imports []*ImportedFile + + // importsBySpecifiedName facilitates the lookup of an ImportedFile given its |SpecifiedName| + importsBySpecifiedName map[string]*ImportedFile // The lexical scope corresponding to this file. FileScope *Scope @@ -58,12 +54,27 @@ Constants []*UserDefinedConstant } +// An ImportedFile represents an element of the "import" list of a .mojom file. +type ImportedFile struct { + // The name as specified in the import statement. + SpecifiedName string + + // The canonical file name of the imported file. This string is the unique identifier for the + // corresponding MojomFile object. Note that when a .mojom file is first parsed + // only the |SpecifiedFileName| of each of its imports is populated because we don't yet know the + // canonical file names of the imported files. It is only when an imported + // file itself is processed that |CanonicalFileName| field is populated within each of the + // importing MojomFiles. + CanonicalFileName string +} + func NewMojomFile(fileName string, descriptor *MojomDescriptor) *MojomFile { mojomFile := new(MojomFile) - mojomFile.FileName = fileName + mojomFile.CanonicalFileName = fileName mojomFile.Descriptor = descriptor mojomFile.ModuleNamespace = "" - mojomFile.Imports = make(map[string]string) + mojomFile.Imports = make([]*ImportedFile, 0) + mojomFile.importsBySpecifiedName = make(map[string]*ImportedFile) mojomFile.Interfaces = make([]*MojomInterface, 0) mojomFile.Structs = make([]*MojomStruct, 0) mojomFile.Unions = make([]*MojomUnion, 0) @@ -73,7 +84,7 @@ } func (f *MojomFile) String() string { - s := fmt.Sprintf("file name: %s\n", f.FileName) + s := fmt.Sprintf("file name: %s\n", f.CanonicalFileName) s += fmt.Sprintf("module: %s\n", f.ModuleNamespace) s += fmt.Sprintf("attributes: %s\n", f.Attributes) s += fmt.Sprintf("imports: %s\n", f.Imports) @@ -90,8 +101,23 @@ return f.FileScope } -func (f *MojomFile) AddImport(fileName string) { - f.Imports[fileName] = "" +func (f *MojomFile) AddImport(specifiedFileName string) { + importedFile := new(ImportedFile) + importedFile.SpecifiedName = specifiedFileName + f.Imports = append(f.Imports, importedFile) + f.importsBySpecifiedName[specifiedFileName] = importedFile +} + +// SetCanonicalImportName sets the |CanonicalFileName| field of the |ImportedFile| +// with the given |SpecifiedName|. This method will usually be invoked later than +// the other methods in this file becuase it is only when the imported file itself +// is processed that we discover its canonical name. +func (f *MojomFile) SetCanonicalImportName(specifiedName, canoncialName string) { + importedFile, ok := f.importsBySpecifiedName[specifiedName] + if !ok { + panic(fmt.Sprintf("There is no imported file with the specifiedName '%s'.", specifiedName)) + } + importedFile.CanonicalFileName = canoncialName } func (f *MojomFile) AddInterface(mojomInterface *MojomInterface) *DuplicateNameError { @@ -139,7 +165,7 @@ // All of the MojomFiles in the order they were visited. mojomFiles []*MojomFile - // All of the MojomFiles keyed by FileName + // All of the MojomFiles keyed by CanonicalFileName MojomFilesByName map[string]*MojomFile // The abstract module namespace scopes keyed by scope name. These are @@ -198,10 +224,10 @@ mojomFile := NewMojomFile(fileName, d) mojomFile.Descriptor = d d.mojomFiles = append(d.mojomFiles, mojomFile) - if _, ok := d.MojomFilesByName[mojomFile.FileName]; ok { - panic(fmt.Sprintf("The file %v has already been processed.", mojomFile.FileName)) + if _, ok := d.MojomFilesByName[mojomFile.CanonicalFileName]; ok { + panic(fmt.Sprintf("The file %v has already been processed.", mojomFile.CanonicalFileName)) } - d.MojomFilesByName[mojomFile.FileName] = mojomFile + d.MojomFilesByName[mojomFile.CanonicalFileName] = mojomFile return mojomFile }
diff --git a/mojom/mojom_parser/mojom/scopes.go b/mojom/mojom_parser/mojom/scopes.go index e7b8afb..346908f 100644 --- a/mojom/mojom_parser/mojom/scopes.go +++ b/mojom/mojom_parser/mojom/scopes.go
@@ -181,7 +181,7 @@ } fileNameString := "" if s.file != nil { - fileNameString = fmt.Sprintf(" in %s", s.file.FileName) + fileNameString = fmt.Sprintf(" in %s", s.file.CanonicalFileName) } return fmt.Sprintf("%s %s%s", s.kind, s.shortName, fileNameString) }
diff --git a/mojom/mojom_parser/mojom/types.go b/mojom/mojom_parser/mojom/types.go index 5eaede0..264aa49 100644 --- a/mojom/mojom_parser/mojom/types.go +++ b/mojom/mojom_parser/mojom/types.go
@@ -580,7 +580,7 @@ if ref.variableAssignment != nil && !ref.resolvedType.IsAssignmentCompatibleWith(ref.variableAssignment.assignedValue) { fileName := "unknown file" if ref.scope != nil && ref.scope.file != nil { - fileName = ref.scope.file.FileName + fileName = ref.scope.file.CanonicalFileName } return fmt.Errorf("Type validation error\n"+ "%s:%s: Illegal assignment: %s %s of type %s may not be assigned the value %v of type %s.", @@ -610,7 +610,7 @@ func (t *UserTypeRef) LongString() string { return fmt.Sprintf("%s %s:%s. (In %s.)", t.identifier, - t.scope.file.FileName, t.token.ShortLocationString(), t.scope) + t.scope.file.CanonicalFileName, t.token.ShortLocationString(), t.scope) } ///////////////////////////////////////////////////////////// @@ -702,7 +702,7 @@ func (v *UserValueRef) LongString() string { return fmt.Sprintf("%s %s:%s. (In %s.)", v.identifier, - v.scope.file.FileName, v.token.ShortLocationString(), v.scope) + v.scope.file.CanonicalFileName, v.token.ShortLocationString(), v.scope) } func NewUserValueRef(assigneeType TypeRef, identifier string, scope *Scope,
diff --git a/mojom/mojom_parser/parser/parse_driver.go b/mojom/mojom_parser/parser/parse_driver.go index 17b6f1d..f178496 100644 --- a/mojom/mojom_parser/parser/parse_driver.go +++ b/mojom/mojom_parser/parser/parse_driver.go
@@ -104,7 +104,7 @@ // Note that we must do this even if the imported file has already been processed // because a given file may be imported by multiple files and each of those need // to be told about the absolute path of the imported file. - currentFile.importedFrom.mojomFile.Imports[currentFile.specifiedPath] = currentFile.absolutePath + currentFile.importedFrom.mojomFile.SetCanonicalImportName(currentFile.specifiedPath, currentFile.absolutePath) } if !descriptor.ContainsFile(currentFile.absolutePath) { @@ -129,14 +129,14 @@ return } currentFile.mojomFile = d.fileExtractor.extractMojomFile(&parser) - for importedFile, _ := range currentFile.mojomFile.Imports { + for _, importedFile := range currentFile.mojomFile.Imports { // Note that it is important that we append all of the imported files here even // if some of them have already been processed. That is because when the imported // file is pulled from the queue it will be pre-processed during which time the // absolute path to the file will be discovered and this absolute path will be // set in |mojomFile| which is necessary for serializing mojomFile. filesToProcess = append(filesToProcess, - &FileReference{importedFrom: currentFile, specifiedPath: importedFile}) + &FileReference{importedFrom: currentFile, specifiedPath: importedFile.SpecifiedName}) } } }
diff --git a/mojom/mojom_parser/parser/parse_driver_test.go b/mojom/mojom_parser/parser/parse_driver_test.go index 9ddcec1..b4880ed 100644 --- a/mojom/mojom_parser/parser/parse_driver_test.go +++ b/mojom/mojom_parser/parser/parse_driver_test.go
@@ -17,7 +17,7 @@ } // FakeFileProvider implements provideContents by recording the name of the file -// whose contents are being requested. +// whose contents are being requested and then returning the empty string. func (f *FakeFileProvider) provideContents(fileRef *FileReference) (contents string, fileReadError error) { f.requestedFileNames = append(f.requestedFileNames, fileRef.specifiedPath) return "", nil @@ -60,7 +60,7 @@ // in the map |importNames|. func (f *FakeFileExtractor) extractMojomFile(parser *Parser) *mojom.MojomFile { file := parser.GetMojomFile() - for _, importName := range f.importNames[file.FileName] { + for _, importName := range f.importNames[file.CanonicalFileName] { file.AddImport(importName) } return file @@ -74,7 +74,7 @@ fakeFileExtractor := makeFakeFileExtractor() // Our fake file1 will import file3, file4, file5 fakeFileExtractor.appendImportsToFile("file1", "file3", "file4", "file5") - // Our fake file5 will import file1 and file5 + // Our fake file5 will import file1 and file6 fakeFileExtractor.appendImportsToFile("file5", "file1", "file6") // Construct the driver under test