Fix for Issue #396 - Duplicate module definitions

[kubajj]

Create a new subroutine called right at the end of the build model subroutine. This subroutine loops through modules provided by each source file and checks for duplicates.

[milancurcic]

Thanks @kubajj! I left a few suggestions.

[LKedward]

Great stuff, thanks Jakub @kubajj, in addition to Milan's comments I spotted a few more things (see my comments).

[kubajj]

During the monthly call, you were talking about rebasing and squashing. Should I do it?

[ivan-pi]

Should this PR include a test case with definition of duplicate modules? @LKedward

[LKedward]

Looking good so far @kubajj! I don't think there's any pressing need to rebase this branch currently.

Should this PR include a test case with definition of duplicate modules? @LKedward

Agreed yes we should put some unit tests in for this.

@kubajj do you want to have a look at test_module_dependencies.f90; this testsuite provides some helpers to mock up package sources very easily and it should be quite straightforward to add a few new tests for this duplicates check. Try to test the various possible scenarios (duplicates in same package, in different packages, in different source types, in same file, no duplicates etc.). Let me know if you need some more explanation of the testsuite. Otherwise I'm happy to also just commit some tests straight to your branch.

[kubajj]

Looking good so far @kubajj! I don't think there's any pressing need to rebase this branch currently.

Should this PR include a test case with definition of duplicate modules? @LKedward

Agreed yes we should put some unit tests in for this.

@kubajj do you want to have a look at test_module_dependencies.f90; this testsuite provides some helpers to mock up package sources very easily and it should be quite straightforward to add a few new tests for this duplicates check. Try to test the various possible scenarios (duplicates in same package, in different packages, in different source types, in same file, no duplicates etc.). Let me know if you need some more explanation of the testsuite. Otherwise I'm happy to also just commit some tests straight to your branch.

@LKedward For creating the different scenarios, should I add source files that would have duplicate modules and load them as in subroutine test_library_module_use or is there another way how to do it?

[LKedward]

No need to write actual fortran source files for these tests, just like the other tests in test_module_dependencies.f90 you can use the new_test_source helper to create a source object with certain modules.

Here's an example unit test to use:

    subroutine test_package_module_duplicates(error)
        
        type(error_t), allocatable, intent(out) :: error

        type(fpm_model_t) :: model
        logical :: duplicates_found

        allocate(model%packages(1))
        allocate(model%packages(1)%sources(2))

        model%packages(1)%sources(1) = new_test_source(FPM_UNIT_MODULE,file_name="src/my_mod_1.f90", &
                                    scope = FPM_SCOPE_LIB, provides=[string_t('my_mod_1')])

        model%packages(1)%sources(2) = new_test_source(FPM_UNIT_MODULE,file_name="src/subdir/my_mod_1.f90", &
                                    scope = FPM_SCOPE_LIB, provides=[string_t('my_mod_1')])

        call check_modules_for_duplicates(model, duplicates_found)
        if (duplicates_found) then
            call test_failed(error,'Duplicate modules found')
            return
        end if
    end subroutine test_package_module_duplicates

(The filename here doesn't matter, the file doesn't need to exist, it is used for other tests.)

Then you just need to add test_package_module_duplicates to the collect routine at the top with should_fail=.true..

[kubajj]

@LKedward Oh, this is brilliant. What did you mean by different source types in the list of possible scenarios?

[LKedward]

Thanks for your work Jakub @kubajj. This all looks good 👍 I've left two minor comments, once those are addressed I think this will be good to merge!

What did you mean by different source types in the list of possible scenarios?

I think I meant like FPM_UNIT_MODULE/FPM_UNIT_PROGRAM etc. but don't worry I don't think that matters for these changes. The tests added here look good as they are.

[LKedward]

I think it would be useful to also print out which file the duplicate has been found in here
(model%packages(k)%sources(l)%file_name)

[LKedward]

Suggested change

	& new_unittest("package-with-duplicates-in-same-source", &
	test_package_module_duplicates_same_source, should_fail=.true.), &

I think these lines have been copy-pasted accidentally

[awvwgk]

Looks good to me.

[awvwgk]

I'll go ahead and merge before we start pruning the Haskell version from this repository.