refactor(cli): move file element validation from mappers to linter#9474
Open
hooni0918 wants to merge 1 commit intotuist:mainfrom
Open
refactor(cli): move file element validation from mappers to linter#9474hooni0918 wants to merge 1 commit intotuist:mainfrom
hooni0918 wants to merge 1 commit intotuist:mainfrom
Conversation
Move file/folder validation logic from manifest mappers to ManifestLinter where it belongs. This removes FIXME comments that indicated the validation should be done in a linter rather than during mapping. Changes: - Make ManifestLinter async with FileSysteming dependency - Add file element validation methods for glob patterns and folder references - Add workspace project path validation - Remove Logger warnings from FileElement, ResourceFileElement, CopyFileElement, and Workspace mappers - Update ManifestGraphLoader to use async linter - Add tests for new file element linting functionality
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Several manifest mappers (FileElement, ResourceFileElement, CopyFileElement, Workspace) contained file validation logic with FIXME comments stating "This should be done in a linter." The validation was happening during model conversion, logging warnings directly via
Logger.current.warning()instead of going through the proper linting system.This made the validation:
LintingIssueWhat
ManifestLinter changes:
ManifestLintingprotocol to async to support file system operationsFileSystemingandRootDirectoryLocatingdependencieslintFileElement()- validates glob patterns and folder referenceslintResourceFileElement()- validates resource file elementslintCopyFileElement()- validates copy file elementslintGlobPattern()- checks if glob matches any files, warns about directories used as globslintFolderReference()- checks folder existence and typelintWorkspaceProjects()- validates workspace project paths existMapper changes:
TuistLoggingimports from FileElement, ResourceFileElement, CopyFileElement, and Workspace mappersLogger.current.warning()calls and FIXME commentsManifestGraphLoader changes:
Test plan
ManifestLinterTests:test_lint_project_fileElementNotFoundtest_lint_project_folderReferenceNotFoundtest_lint_project_folderReferenceIsNotDirectorytest_lint_project_directoryUsedAsGlobtest_lint_project_validFileElementtest_lint_project_validFolderReferencetuist generateon a project with missing file paths and verify warnings appear