Make ProjectModule opaque

This commit is contained in:
Jeroen Engels 2023-02-01 22:55:07 +01:00
parent e628d4ba57
commit f4ef7c2a3a
6 changed files with 136 additions and 62 deletions

View File

@ -21,7 +21,7 @@ import NonEmpty exposing (NonEmpty)
import Review.ModuleNameLookupTable.Internal as ModuleNameLookupTableInternal exposing (ModuleNameLookupTable)
import Review.Project.Dependency
import Review.Project.ProjectCache as ProjectCache exposing (ProjectCache)
import Review.Project.ProjectModule exposing (ProjectModule)
import Review.Project.ProjectModule as ProjectModule exposing (ProjectModule)
import Review.Project.Valid as ValidProject exposing (ValidProject)
import Set exposing (Set)
import Vendor.ListExtra as ListExtra
@ -103,15 +103,19 @@ compute moduleName module_ project =
modulesByModuleName =
ValidProject.modulesByModuleName project
moduleAst : Elm.Syntax.File.File
moduleAst =
ProjectModule.ast module_
( imported, projectCacheWithComputedImports ) =
List.foldl
(computeImportedModulesDocs modulesByModuleName deps)
( Dict.empty, projectCache )
(elmCorePrelude ++ module_.ast.imports)
(elmCorePrelude ++ moduleAst.imports)
cacheKey : ProjectCache.ModuleCacheKey
cacheKey =
{ imported = imported, source = module_.source }
{ imported = imported, source = ProjectModule.source module_ }
computeLookupTableForModule : () -> ( ModuleNameLookupTable, Dict ModuleName Elm.Docs.Module )
computeLookupTableForModule () =
@ -119,8 +123,8 @@ compute moduleName module_ project =
moduleContext : Context
moduleContext =
fromProjectToModule moduleName imported
|> collectModuleDocs module_.ast
|> collectLookupTable module_.ast.declarations
|> collectModuleDocs moduleAst
|> collectLookupTable moduleAst.declarations
in
( moduleContext.lookupTable
, Dict.insert moduleName
@ -165,16 +169,20 @@ computeOnlyModuleDocs :
-> ( Elm.Docs.Module, ProjectCache )
computeOnlyModuleDocs moduleName module_ modulesByModuleName deps projectCache =
let
moduleAst : Elm.Syntax.File.File
moduleAst =
ProjectModule.ast module_
( imported, projectCacheWithComputedImports ) =
List.foldl
(computeImportedModulesDocs modulesByModuleName deps)
( Dict.empty, projectCache )
(elmCorePrelude ++ module_.ast.imports)
(elmCorePrelude ++ moduleAst.imports)
moduleContext : Context
moduleContext =
fromProjectToModule moduleName imported
|> collectModuleDocs module_.ast
|> collectModuleDocs moduleAst
moduleDocs : Elm.Docs.Module
moduleDocs =

View File

@ -160,7 +160,7 @@ addParsedModule { path, source, ast } project =
addModuleToProject : ProjectModule -> Project -> Project
addModuleToProject module_ (Internal.Project project) =
Internal.Project { project | modules = Dict.insert module_.path module_ project.modules }
Internal.Project { project | modules = Dict.insert (ProjectModule.path module_) module_ project.modules }
addFileThatFailedToParse : { path : String, source : String } -> Project -> Project
@ -245,13 +245,15 @@ addElmJson elmJson_ (Internal.Project project) =
else
Dict.map
(\_ value ->
(\path module_ ->
let
osAgnosticPath : String
osAgnosticPath =
Path.makeOSAgnostic value.path
Path.makeOSAgnostic path
in
{ value | isInSourceDirectories = List.any (\dir -> String.startsWith dir osAgnosticPath) sourceDirectories }
ProjectModule.setIsInSourceDirectories
(List.any (\dir -> String.startsWith dir osAgnosticPath) sourceDirectories)
module_
)
project.modules
in

View File

@ -1,6 +1,16 @@
module Review.Project.ProjectModule exposing (ProjectModule, create)
module Review.Project.ProjectModule exposing
( ProjectModule, create
, path, source, ast, contentHash, isInSourceDirectories
, setIsInSourceDirectories
)
{-| Represents a parsed file.
@docs ProjectModule, create
@docs path, source, ast, contentHash, isInSourceDirectories
@docs setIsInSourceDirectories
-}
import Elm.Syntax.File
@ -8,13 +18,14 @@ import Elm.Syntax.Node exposing (Node(..))
import Review.Cache.ContentHash as ContentHash exposing (ContentHash)
type alias ProjectModule =
{ path : String
, source : String
, ast : Elm.Syntax.File.File
, contentHash : ContentHash
, isInSourceDirectories : Bool
}
type ProjectModule
= ProjectModule
{ path : String
, source : String
, ast : Elm.Syntax.File.File
, contentHash : ContentHash
, isInSourceDirectories : Bool
}
create :
@ -25,17 +36,18 @@ create :
}
-> ProjectModule
create params =
{ path = params.path
, source = params.source
, ast = sanitizeModule params.ast
, contentHash = ContentHash.hash params.source
, isInSourceDirectories = params.isInSourceDirectories
}
ProjectModule
{ path = params.path
, source = params.source
, ast = sanitizeModule params.ast
, contentHash = ContentHash.hash params.source
, isInSourceDirectories = params.isInSourceDirectories
}
sanitizeModule : Elm.Syntax.File.File -> Elm.Syntax.File.File
sanitizeModule ast =
{ ast | comments = List.sortBy (\(Node range _) -> positionAsInt range.start) ast.comments }
sanitizeModule ast_ =
{ ast_ | comments = List.sortBy (\(Node range _) -> positionAsInt range.start) ast_.comments }
positionAsInt : { row : Int, column : Int } -> Int
@ -44,3 +56,33 @@ positionAsInt { row, column } =
-- It is entirely based on the assumption that no line is longer than
-- 1.000.000 characters long, which the compiler does not support for Elm 0.19.1.
row * 1000000 + column
path : ProjectModule -> String
path (ProjectModule module_) =
module_.path
source : ProjectModule -> String
source (ProjectModule module_) =
module_.source
ast : ProjectModule -> Elm.Syntax.File.File
ast (ProjectModule module_) =
module_.ast
contentHash : ProjectModule -> ContentHash
contentHash (ProjectModule module_) =
module_.contentHash
isInSourceDirectories : ProjectModule -> Bool
isInSourceDirectories (ProjectModule module_) =
module_.isInSourceDirectories
setIsInSourceDirectories : Bool -> ProjectModule -> ProjectModule
setIsInSourceDirectories isInSourceDirectories_ (ProjectModule module_) =
ProjectModule { module_ | isInSourceDirectories = isInSourceDirectories_ }

View File

@ -203,11 +203,15 @@ duplicateModuleNames visitedModules projectModules =
moduleName : ModuleName
moduleName =
getModuleName projectModule
projectModulePath : String
projectModulePath =
ProjectModule.path projectModule
in
case Dict.get moduleName visitedModules of
Nothing ->
duplicateModuleNames
(Dict.insert moduleName projectModule.path visitedModules)
(Dict.insert moduleName projectModulePath visitedModules)
restOfModules
Just path ->
@ -215,10 +219,10 @@ duplicateModuleNames visitedModules projectModules =
{ moduleName = moduleName
, paths =
path
:: projectModule.path
:: projectModulePath
:: (restOfModules
|> List.filter (\p -> getModuleName p == moduleName)
|> List.map .path
|> List.map ProjectModule.path
)
}
@ -270,7 +274,7 @@ buildModuleGraph mods =
nodesAndEdges : (ModuleName -> Maybe Int) -> ProjectModule -> Int -> ( Graph.Node FilePath, List (Graph.Edge ()) )
nodesAndEdges getModuleId module_ moduleId =
( Graph.Node moduleId module_.path
( Graph.Node moduleId (ProjectModule.path module_)
, importedModules module_
|> List.filterMap getModuleId
|> List.map
@ -282,13 +286,13 @@ nodesAndEdges getModuleId module_ moduleId =
importedModules : ProjectModule -> List ModuleName
importedModules module_ =
module_.ast.imports
(ProjectModule.ast module_).imports
|> List.map (Node.value >> .moduleName >> Node.value)
getModuleName : ProjectModule -> ModuleName
getModuleName module_ =
module_.ast.moduleDefinition
(ProjectModule.ast module_).moduleDefinition
|> Node.value
|> Elm.Syntax.Module.moduleName
@ -398,7 +402,7 @@ addParsedModule { path, source, ast } maybeModuleZipper (ValidProject project) =
newProject =
{ project | modulesByPath = Dict.insert path module_ project.modulesByPath }
in
if importedModulesSet existingModule.ast project.dependencyModules == importedModulesSet ast project.dependencyModules then
if importedModulesSet (ProjectModule.ast existingModule) project.dependencyModules == importedModulesSet ast project.dependencyModules then
let
-- Imports haven't changed, we don't need to recompute the zipper or the graph
newModuleZipper : Zipper (Graph.NodeContext FilePath ())

View File

@ -327,6 +327,7 @@ import Review.Project exposing (ProjectModule)
import Review.Project.Dependency
import Review.Project.Internal exposing (Project)
import Review.Project.InvalidProjectError as InvalidProjectError
import Review.Project.ProjectModule as ProjectModule
import Review.Project.Valid as ValidProject exposing (ValidProject)
import Review.RequestedData as RequestedData exposing (RequestedData(..))
import Vendor.Graph as Graph exposing (Graph)
@ -2064,7 +2065,7 @@ withContextFromImportedModules (ProjectRuleSchema schema) =
setFilePathIfUnset : ProjectModule -> Error scope -> Error scope
setFilePathIfUnset module_ ((Error err) as rawError) =
if err.filePath == "" then
Error { err | filePath = module_.path }
Error { err | filePath = ProjectModule.path module_ }
else
rawError
@ -4844,7 +4845,7 @@ computeModule ({ dataToComputeModules, module_, isFileIgnored, projectContext, p
moduleName : ModuleName
moduleName =
Node.value (moduleNameNode module_.ast.moduleDefinition)
Node.value (moduleNameNode (ProjectModule.ast module_).moduleDefinition)
( moduleNameLookupTable, newProject ) =
if requestedData.moduleNameLookupTable then
@ -4855,17 +4856,17 @@ computeModule ({ dataToComputeModules, module_, isFileIgnored, projectContext, p
availableData : AvailableData
availableData =
{ ast = module_.ast
, moduleKey = ModuleKey module_.path
{ ast = ProjectModule.ast module_
, moduleKey = ModuleKey (ProjectModule.path module_)
, moduleNameLookupTable = moduleNameLookupTable
, extractSourceCode =
if requestedData.sourceCodeExtractor then
extractSourceCode (String.lines module_.source)
extractSourceCode (String.lines (ProjectModule.source module_))
else
always ""
, filePath = module_.path
, isInSourceDirectories = module_.isInSourceDirectories
, filePath = ProjectModule.path module_
, isInSourceDirectories = ProjectModule.isInSourceDirectories module_
, isFileIgnored = isFileIgnored
}
@ -4909,7 +4910,7 @@ findFixInComputeModuleResults ({ dataToComputeModules, module_, isFileIgnored, p
analysis : ModuleCacheEntry projectContext
analysis =
Cache.createModuleEntry
{ contentHash = module_.contentHash
{ contentHash = ProjectModule.contentHash module_
, errors = errors
, inputContext = projectContext
, isFileIgnored = isFileIgnored
@ -4950,10 +4951,16 @@ findFixInComputeModuleResults ({ dataToComputeModules, module_, isFileIgnored, p
filePath =
errorFilePath fixResult.error
in
if module_.path == filePath then
if ProjectModule.path module_ == filePath then
ReComputeModule
{ params
| module_ = { module_ | source = source, ast = ast }
| module_ =
ProjectModule.create
{ path = filePath
, source = source
, ast = ast
, isInSourceDirectories = ProjectModule.isInSourceDirectories module_
}
, project = fixResult.project
, moduleZipper = newModuleZipper_
, fixedErrors = newFixedErrors
@ -5109,7 +5116,12 @@ computeModuleAndCacheResult dataToComputeModules inputProjectContext moduleZippe
ignoreModule ()
Just module_ ->
if shouldIgnoreModule dataToComputeModules module_.path then
let
modulePath : String
modulePath =
ProjectModule.path module_
in
if shouldIgnoreModule dataToComputeModules modulePath then
ignoreModule ()
else
@ -5123,12 +5135,12 @@ computeModuleAndCacheResult dataToComputeModules inputProjectContext moduleZippe
isFileIgnored : Bool
isFileIgnored =
not (Exceptions.isFileWeWantReportsFor dataToComputeModules.exceptions module_.path)
not (Exceptions.isFileWeWantReportsFor dataToComputeModules.exceptions modulePath)
shouldReuseCache : Cache.ModuleEntry error projectContext -> Bool
shouldReuseCache cacheEntry =
Cache.match
module_.contentHash
(ProjectModule.contentHash module_)
(ContextHash.create projectContext)
cacheEntry
{ isFileIgnored = isFileIgnored
@ -5137,7 +5149,7 @@ computeModuleAndCacheResult dataToComputeModules inputProjectContext moduleZippe
maybeCacheEntry : Maybe (ModuleCacheEntry projectContext)
maybeCacheEntry =
Dict.get module_.path moduleContexts
Dict.get modulePath moduleContexts
in
case reuseCache shouldReuseCache maybeCacheEntry of
Just cacheEntry ->
@ -5200,7 +5212,7 @@ computeModuleAndCacheResult dataToComputeModules inputProjectContext moduleZippe
}
in
{ project = result.project
, moduleContexts = Dict.insert module_.path result.analysis moduleContexts
, moduleContexts = Dict.insert modulePath result.analysis moduleContexts
, nextStep = result.nextStep
, fixedErrors = result.fixedErrors
}
@ -5301,7 +5313,7 @@ findFixHelp project fixablePredicate errors maybeModuleZipper =
Just file ->
case
InternalFix.fixModule fixes file.source
InternalFix.fixModule fixes (ProjectModule.source file)
|> Maybe.andThen
(\fixResult ->
ValidProject.addParsedModule { path = headError.filePath, source = fixResult.source, ast = fixResult.ast } maybeModuleZipper project
@ -5382,14 +5394,19 @@ isFixable predicate err =
visitModuleForProjectRule : RunnableModuleVisitor moduleContext -> moduleContext -> ProjectModule -> ( List (Error {}), moduleContext )
visitModuleForProjectRule schema initialContext module_ =
let
ast : File
ast =
ProjectModule.ast module_
in
( [], initialContext )
|> accumulateWithListOfVisitors schema.moduleDefinitionVisitors module_.ast.moduleDefinition
|> accumulateWithListOfVisitors schema.moduleDefinitionVisitors ast.moduleDefinition
-- TODO When `elm-syntax` integrates the module documentation by default, then we should use that instead of this.
|> accumulateModuleDocumentationVisitor schema.moduleDocumentationVisitors module_.ast
|> accumulateWithListOfVisitors schema.commentsVisitors module_.ast.comments
|> accumulateList schema.importVisitors module_.ast.imports
|> accumulateWithListOfVisitors schema.declarationListVisitors module_.ast.declarations
|> schema.declarationAndExpressionVisitor module_.ast.declarations
|> accumulateModuleDocumentationVisitor schema.moduleDocumentationVisitors ast
|> accumulateWithListOfVisitors schema.commentsVisitors ast.comments
|> accumulateList schema.importVisitors ast.imports
|> accumulateWithListOfVisitors schema.declarationListVisitors ast.declarations
|> schema.declarationAndExpressionVisitor ast.declarations
|> (\( errors, moduleContext ) -> ( makeFinalModuleEvaluation schema.finalEvaluationFns errors moduleContext, moduleContext ))

View File

@ -137,6 +137,7 @@ import Review.FileParser as FileParser
import Review.Fix as Fix
import Review.Options as ReviewOptions
import Review.Project as Project exposing (Project, ProjectModule)
import Review.Project.ProjectModule as ProjectModule
import Review.Rule as Rule exposing (ReviewError, Rule)
import Review.Test.Dependencies exposing (projectWithElmCore)
import Review.Test.FailureMessage as FailureMessage
@ -494,14 +495,14 @@ hasOneElement list =
moduleToRunResult : List ReviewError -> ProjectModule -> SuccessfulRunResult
moduleToRunResult errors projectModule =
{ moduleName =
projectModule.ast.moduleDefinition
(ProjectModule.ast projectModule).moduleDefinition
|> Node.value
|> Module.moduleName
|> String.join "."
, inspector = codeInspectorForSource True projectModule.source
, inspector = codeInspectorForSource True (ProjectModule.source projectModule)
, errors =
errors
|> List.filter (\error_ -> Rule.errorFilePath error_ == projectModule.path)
|> List.filter (\error_ -> Rule.errorFilePath error_ == ProjectModule.path projectModule)
|> List.sortWith compareErrorPositions
}
@ -578,11 +579,11 @@ findDuplicateModuleNames previousModuleNames modules =
[] ->
Nothing
{ ast } :: restOfModules ->
module_ :: restOfModules ->
let
moduleName : List String
moduleName =
ast.moduleDefinition
(ProjectModule.ast module_).moduleDefinition
|> Node.value
|> Module.moduleName
in
@ -887,7 +888,7 @@ doCheckResultsAreTheSameWhenIgnoringFiles allErrors rule project =
filePaths : List String
filePaths =
Project.modules project
|> List.map .path
|> List.map ProjectModule.path
|> maybeCons .path (Project.elmJson project)
|> maybeCons .path (Project.readme project)