Ensure users don't create a .wasp file

[komyg]

Description

Fixes: #2406

Select what type of change this PR introduces:

1. Just code/docs improvement (no functional change).
2. Bug fix (non-breaking change which fixes an issue).
3. New feature (non-breaking change which adds functionality).
4. Breaking change (fix or feature that would cause existing functionality to not work as expected).

Update Waspc ChangeLog and version if needed

If you did a bug fix, new feature, or breaking change, that affects waspc, make sure you satisfy the following:

1. I updated ChangeLog.md with description of the change this PR introduces.
2. I bumped waspc version in waspc.cabal to reflect changes I introduced, with regards to the version of the latest wasp release, if the bump was needed.

Update example apps if needed

If you did code changes and added a new feature or modified an existing feature, make sure you satisfy the following:

1. I updated waspc/examples/todoApp as needed (updated modified feature or added new feature) and manually checked it works correctly.
2. I updated waspc/headless-test/examples/todoApp and its e2e tests as needed (updated modified feature and its tests or added new feature and new tests for it).

[sodic]

Hey @komyg, thanks for the effort!

You have the right idea, and I see you know how to use both Haskell and our codebase (props for using relfile and IOUtil.doesFileExist), so I have no doubt we'll be merging this soon.

There are some improvements we can do, I left three requests (the biggest of which is moving the logic into findWaspFile).

Let me know if you have any questions :)

[sodic]

This function is called waspDirExists, implying that it checks whether a .wasp directory exists and returns either an error or a path to the directory.

But, in reality, the function does something else:

• If a .wasp directory does exist, it returns a path to the existing .wasp directory (so far so good)
• If the .wasp directory doesn't exist, it returns a path to the non-existing .wasp directory (this is the first weird part)
• If a .wasp file exists, it returns an error saying that .wasp is a file (but the function's name implies it should only care about whether the .wasp directory exists and report on that).

Imagine what happens when someone else tries to use this function in a different context (e.g., to really check whether the .wasp directory exists). I think they'd end up very confused :)

Anyway, for this issue, we don't care whether the .wasp directory exists or not. It can exist, it doesn't need to exist - it's all the same to us.

We only care about whether a .wasp file exists, so we should write a function to check for that, and name it accordingly.

[komyg]

Yes, you are right.

My initial idea here, was to make this function return the waspDir as a right value (in case there was no .wasp file), or an error as a left value.

Then I would send the right result to the next function call: waspFilePathOrError <- left (: []) <$> findWaspFile waspDir, because that function takes the waspDir as an argument.

However, I didn't end up doing this, and in the end I should just have returned a boolean or empty right value.

[komyg]

My original plan for this implementation was to create a sort of pipeline in which the result of the previous function would go into the next function, as it is usually done in railway oriented programming.

This way, I would be able to avoid having two case statements in the analyzeWaspProject, because the next waspFilePathOrError would only be called if its input was a right value, otherwise it will pass through the left value unchanged.

Unfortunately, I was unable to do that, and I forgot to change the return type of this function to boolean or an empty right value.

[sodic]

Understood!

Thanks for sharing your thoughts. I enjoyed reading that article.

[sodic]

The "Wasp project" is the user's project, not the stuff in the .wasp directory.

Also, users don't really care about the .wasp directory, so this message gives out unnecessary information. If I read it, I'd think Wasp expects me to create a .wasp directory (which isn't true).

We should tell users something that conveys the message "Hey, your .wasp file can't be called .wasp, it needs to have a proper name," no need to mention the .wasp directory at all.

[komyg]

Ok, I've re-phrased the message as: Invalid file name for the .wasp file. Please rename it to [something].wasp.. What do you think?

[sodic]

I like it, I'd probably say:

Your Wasp file can't be called just '.wasp'. Please rename it to [something].wasp

It's a little more direct and tells you exactly what the problem is. But we can keep your message if you prefer it.

[sodic]

A better place to handle this check is the function findWaspFile.

After all, we don't care about the directory at all. All we care about is:

• Do the users have a *.wasp file?
• If they do, is it called something other than just .wasp?
• If not, tell them to name it properly.

That's on me, the issue could have been explained better. Sorry for the troubles :)

So, to summarize one last time:

While looking for a *.wasp file, we should also ensure that the found file isn't called .wasp

While you're at it, you might also want to check whether there are multiple .wasp files and report that.

Because, in theory, Wasp will only find one of *.wasp files. Let's say we have two of them: something.wasp and .wasp. findWaspFile could return the something.wasp, letting .wasp slip under the radar and cause problems later.

And even if it didn't, allowing multiple .wasp files can be confusing.

[komyg]

Hey @sodic, thank you for your review, it was very through!

I believe I addressed most of your comments. Could you review again, please?

The only thing left is making sure we don't have more than one .wasp file. I am currently working on that, but it is not done yet. If I can get it done before your next review, then I can be a part of this PR, otherwise, I can make it into another PR. What do you think?

[komyg]

Hey @sodic, I managed to check if we have more than one *.wasp or *.wasp.ts files in the .wasp folder. Could you review again and tell me what you think, please?

[sodic]

Nice work, Felipe!

I appreciate the patience. I tried to make it up to you with a thorough review.

Let me know what you think!

[sodic]

Nice!

This works, but, instead of having two IO procedures (doesFileExist and listDirectory) that share implicit information, it's better to have a single IO call and check all the constraints on its result.

More precisely, the logic after IOUtil.listDirectory assumes someone has already checked the .wasp file situation, but this knowledge isn't communicated through the type system. This isn't too bad (it's all in the same function after all), but we could improve it.

So, let's try to keep all the IO querying at the start of the function, and all the logic at its end.

I'll help you out here (since StrongPath can be difficult to deal with :).

Here's a rough sketch of what I had in mind:

findWaspFile :: Path' Abs (Dir WaspProjectDir) -> IO (Either String WaspFilePath)
findWaspFile projectDir = do
  filesInProjectDir <- fst <$> IOUtil.listDirectory projectDir
  let filesEndingWithWasp = findAllFilesWithSuffix ".wasp" filesInProjectDir
      filesEndingWithWaspTs = findAllFilesWithSuffix ".wasp.ts" filesInProjectDir
  return $ case (filesEndingWithWasp, filesEndingWithWaspTs) of
    -- We'll check everyhing here
    
 
-- You can place this function in Wasp.Util.StrongPath, it might be useful elsewhere :)
findAllFilesWithSuffix :: String -> [Path p r (File f)] -> [Path p r (File f)]
findAllFilesWithSuffix extension = filter ((extension `isSuffixOf`) . toFilePath)

The proposed separation carries an extra benefit. If all the IO stuff is grouped together, it's easy to extract all the logic into a pure function and test it with unit tests.

You can do this too if you want, but don't feel like you have to :)

[sodic]

This is a very nice display of Haskell's power!

That said, I'd probably stick to pattern matching as much as possible (as opposed to conditional checks) - they're more declarative and better at describing mappings that don't involve too much logic.

Here's what I had in mind:

return $ case (filesEndingWithWasp, filesEndingWithWaspTs) of
  ([], []) -> Left fileNotFoundMessage
  (_ : _, _ : _) -> Left bothFilesFoundMessage
  ([], waspTsFiles) -> case waspTsFiles of
    [singleWaspTsFile] -> Right . WaspTs $ castFile (projectDir </> singleWaspTsFile)
    multipleWaspTsFiles -> -- ...
  (waspLangFiles, []) -> case waspLangFiles of
    [singleWaspLangFile]
      | toFilePath (basename singleWaspLangFile) == ".wasp" -> -- ... Double check whether this works please
      | otherwise -> Right . WaspLang $ castFile (projectDir </> singleWaspLangFile)
    multipleWaspLangFiles -> -- ...

With pattern matches, Haskell ensures you've covered all the options. If you use conditional checks (e.g., null tsFiles, [waspTsFile] <- tsFiles, ...), the compiler can't check their exhaustiveness.

[sodic]

Understood!

Thanks for sharing your thoughts. I enjoyed reading that article.

[sodic]

I like it, I'd probably say:

Your Wasp file can't be called just '.wasp'. Please rename it to [something].wasp

It's a little more direct and tells you exactly what the problem is. But we can keep your message if you prefer it.