Catch module+program or multiple modules in a source file and print a helpful message to the user

[milancurcic]

See #126. fpm currently allows either a single module or a single program in a source file. However, Fortran allows having:

• Multiple modules per source file
• Module + program in a source file
• Procedures without modules in a source file

fpm should catch these scenarios and print a helpful error message for the user.

[arjenmarkus]

Just a comment: I noticed that fpm does allow more than one module in a file - and it seems to work: one of the source files I experimented with has a small module to define a type and then a module with the same name as the source file that uses it. So not two independent ones, I agree.

[certik]

For simplicity, I think by default fpm should simply expect one module per file, and impose the naming consistency convention. We can discuss if we should implement optional options in fpm.toml to allow more modules per file.

[awvwgk]

I think this has been fixed already.

[certik]

I tried the latest master and unfortunately there is no error or warning printed when more than one module is present in a file. How to reproduce:

fpm new xx
cd xx

apply the following patch:

--- a/src/xx.f90
+++ b/src/xx.f90
@@ -8,3 +8,14 @@ contains
     print *, "Hello, xx!"
   end subroutine say_hello
 end module xx
+
+module yy
+  implicit none
+  private
+
+  public :: say_hello
+contains
+  subroutine say_hello
+    print *, "Hello, xx!"
+  end subroutine say_hello
+end module yy

then compile:

$ fpm build
 + mkdir -p build/dependencies
 + mkdir -p build/gfortran_2A42023B310FA28D/xx
 + gfortran -c test/check.f90 -Wall -Wextra -Wimplicit-interface -fPIC -fmax-errors=1 -g -fcheck=bounds -fcheck=array-temps -fbacktrace -fcoarray=single -J build/gfortran_2A42023B310FA28D/xx -I build/gfortran_2A42023B310FA28D/xx  -o build/gfortran_2A42023B310FA28D/xx/test_check.f90.o
 + gfortran -c ././src/xx.f90 -Wall -Wextra -Wimplicit-interface -fPIC -fmax-errors=1 -g -fcheck=bounds -fcheck=array-temps -fbacktrace -fcoarray=single -J build/gfortran_2A42023B310FA28D/xx -I build/gfortran_2A42023B310FA28D/xx  -o build/gfortran_2A42023B310FA28D/xx/src_xx.f90.o
 + ar -rs build/gfortran_2A42023B310FA28D/xx/libxx.a build/gfortran_2A42023B310FA28D/xx/src_xx.f90.o
ar: creating archive build/gfortran_2A42023B310FA28D/xx/libxx.a
 + gfortran -c app/main.f90 -Wall -Wextra -Wimplicit-interface -fPIC -fmax-errors=1 -g -fcheck=bounds -fcheck=array-temps -fbacktrace -fcoarray=single -J build/gfortran_2A42023B310FA28D/xx -I build/gfortran_2A42023B310FA28D/xx  -o build/gfortran_2A42023B310FA28D/xx/app_main.f90.o
 + mkdir -p build/gfortran_2A42023B310FA28D/app/
 + gfortran  -Wall -Wextra -Wimplicit-interface -fPIC -fmax-errors=1 -g -fcheck=bounds -fcheck=array-temps -fbacktrace -fcoarray=single -J build/gfortran_2A42023B310FA28D/xx -I build/gfortran_2A42023B310FA28D/xx  build/gfortran_2A42023B310FA28D/xx/app_main.f90.o build/gfortran_2A42023B310FA28D/xx/libxx.a -o build/gfortran_2A42023B310FA28D/app/xx
 + mkdir -p build/gfortran_2A42023B310FA28D/test/
 + gfortran  -Wall -Wextra -Wimplicit-interface -fPIC -fmax-errors=1 -g -fcheck=bounds -fcheck=array-temps -fbacktrace -fcoarray=single -J build/gfortran_2A42023B310FA28D/xx -I build/gfortran_2A42023B310FA28D/xx  build/gfortran_2A42023B310FA28D/xx/test_check.f90.o build/gfortran_2A42023B310FA28D/xx/libxx.a -o build/gfortran_2A42023B310FA28D/test/check

No warning and no error.

We need to be printing warnings (and even errors) every time:

• More than one module or one program is present in a file
• The module name does not follow the convention of prepending the package name in the module name

And both can be overiden in fpm.toml if your package follows a different convention.

[awvwgk]

We need to be printing warnings (and even errors) every time:

• More than one module or one program is present in a file
• The module name does not follow the convention of prepending the package name in the module name

And both can be overiden in fpm.toml if your package follows a different convention.

A program and a module in the same scope seems completely reasonable to me. Multiple modules less so, still there are use cases for this (example). We have to be careful to not break existing projects or fpm itself when introducing this change. Therefore, this check should at first be opt-in for all projects, we can later make it the default once it found adoption.

For example in TOML Fortran the package name toml-f does not cleanly map to the used prefix tomlf_. Same holds true for other projects that use a dash in the package name or a different prefix for the modules than the package name.

To handle projects that can't map their package name to a prefix there should be an option in library:

name = "toml-f"

[library]
namespace = "tomlf"  # or maybe prefix?

Regarding the opt-in/opt-out of this convention, what table is best suited for this purpose? I wouldn't put it under the top-level and build also does not seem fitting here.

Finally, the namespace rules doesn't really apply for app/test/example, does it? Those modules are private within the project and can therefore have a less strict naming convention.

[certik]

Yes, we definitely need to be careful and we don't want to break things.

This is why it is very important to do these "compliance requirements" early in fpm's life. That means now. We can always relax them without breaking anything. But we cannot make them more strict.

We don't need to require this restriction of just one program/module per file. But it seems like a good habit. The same with the naming convention. @everythingfunctional and I even brainstormed being more strict on module names and encode the path in them, say in fpm's src/fpm/manifest/build.f90 module would be named fpm_manifest_build. In fact that is precisely how it is already named! So fpm enforcing it would be very natural. This could of course be disabled. However, I can't seem to convince others this is a good idea, so that's all right.

One way we can enforce these things is with opt in options, that would be enabled by default for new projects (fpm new) in their fpm.toml file explicitly. That would not break anything. Finally, since there could be quite a few of these "new restrictions", we can even lump them together and do something like

defaults = "fpm 0.4.0"

Which would enable (on opt-in basis) any defaults that fpm 0.4.0 likes as defaults. Then fpm 0.5.0 can have different defaults, and things would not break.

[milancurcic]

I don't recall what we decided about this, but I think it was this: To protect against module name clashes between the project modules and those of dependencies, a naming convention needs to be enforced to ensure unique module names. @certik describes that convention above.

However, I can't seem to convince others this is a good idea, so that's all right.

I don't think this is what's happening; I think it's more that it just hasn't been implemented. I'm in favor of this, for example. I also vaguely recall us having an overall consensus about this, though I can't find the thread.

At the same time, we should also allow building valid Fortran code as much as the convention allows it. Is it possible to design a naming convention that would allow multiple-modules-per-source-file and modules+program in a source file?

How about this:

• Source file src/fpm/manifest/build.f90 must contain a module named fpm_manifest_build
• Source file src/fpm/manifest/build.f90 may contain additional modules named fpm_manifest_build_<suffix>; if it does, src/fpm/manifest will not also contain a directory src/fpm/manifest/build.
• Source file app/main.f90 (or any other source file under app/) may contain a program and a module, if the module is named <package-name>_<suffix>, where suffix is not already in use by modules/source files/directories in src/.

And if this works, we should still detect and print a helpful warning message a-la:

Warning: Multiple modules defined in src/fpm/manifest/build.f90; Consider defining each module in its own source file.

For the program+module pattern which I enjoy very much, I wouldn't even print a warning. But that's just my preference; I wouldn't strongly oppose warnings for that.

[certik]

There are two conventions:

• Prefixing all modules just with the project names. I think there is consensus on this (even if it is not implemented and enforced yet).
• Naming the module based on file name. That convention has been there I believe, but it was removed, although I can't find the PR right now. I don't think there is consensus on this. But I still like it, and I am happy that you do too.

I like your thinking about multiple modules in a file. I also use module+main program in the same file sometimes. So I am not against it. As long as fpm can know exactly what is going on and what is where. I think it can.

[everythingfunctional]

I'm also in favor of enforcing (with a light touch if possible, i.e. optional) a one module per file and naming convention rule, as it seems like a pretty clear best practice. I think we don't need such restrictions on app, example or test code, because it seems quite frequent to want to put program+modules in a single source file for the purposes of quick one-off and demonstration code.

[certik]

When we depend on a library, it's mainly the src/ directory that gets made "available" to use? Or are app, example and test also somehow installed?

If so, it would make sense to be strict with the src/ modules (one module per file, strict naming convention based on the name of the package and path).

[LKedward]

As previously discussed I agree with requiring project module prefixes as this is our only option for some form of name-spacing, however I strongly disagree with the need to invent and enforce a convention for naming modules based on file path.

IMO, such a path-module convention would greatly restrict the ease and flexibility of using fpm as it is now. I don't see any concrete advantage to creating such a convention, but the cost in terms of project maintenance is quite significant. Consider the simple task of restructuring a growing project into subfolders: this would require manually renaming modules and renaming all uses of them in other modules/projects/programs.

This would also greatly complicate the task of converting existing packages to fpm since all modules would require renaming. I really do not think we should pursue such a rigid convention within fpm itself, even as an optional feature, but rather leave it up to project maintainers to use a good naming convention.

[certik]

Good point about renaming. Note that you have a similar issue in Python. If you move things around, the imports will break, so you have to fix them. If Fortran introduces some kind of namespaces, based on path, it would have exactly the same issue.

Converting existing packages to fpm does not change as you would be able to just disable this check with a single line in fpm.toml. It can also just be a warning. That way things still compile.