Merge pull request #287 from wravery/sonar-lint

Export compile commands and fix some SonarLint warnings

Author: wravery

CMakeLists.txt

@@ -9,6 +9,9 @@ cmake_policy(SET CMP0091 NEW)
 # Do not set default MSVC warning flags: https://cmake.org/cmake/help/latest/policy/CMP0092.html
 cmake_policy(SET CMP0092 NEW)
 
+# Export compile commands for other tools, e.g. SonarLint.
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
+
 # Default to the last updated version from version.txt.
 file(TO_CMAKE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/version.txt" VERSION_FILE)
 file(READ "${VERSION_FILE}" LATEST_VERSION)

doc/awaitable.md

@@ -8,14 +8,14 @@ specifically a type-erased `graphql::service::await_async` class in
 [GraphQLService.h](../include/graphqlservice/GraphQLService.h):
 ```cpp
 // Type-erased awaitable.
-class [[nodiscard]] await_async final
+class [[nodiscard("unnecessary construction")]] await_async final
 {
 private:
-	struct [[nodiscard]] Concept
+	struct [[nodiscard("unnecessary construction")]] Concept
 	{
 		virtual ~Concept() = default;
 
-		[[nodiscard]] virtual bool await_ready() const = 0;
+		[[nodiscard("unexpected call")]] virtual bool await_ready() const = 0;
 		virtual void await_suspend(coro::coroutine_handle<> h) const = 0;
 		virtual void await_resume() const = 0;
 	};
@@ -71,22 +71,22 @@ Many APIs which used to return some sort of `std::future` now return an alias fo
 `graphql::internal::Awaitable<...>`. This template is defined in [Awaitable.h](../include/graphqlservice/internal/Awaitable.h):
 ```cpp
 template <typename T>
-class [[nodiscard]] Awaitable
+class [[nodiscard("unnecessary construction")]] Awaitable
 {
 public:
 	Awaitable(std::future<T> value)
 		: _value { std::move(value) }
 	{
 	}
 
-	[[nodiscard]] T get()
+	[[nodiscard("unnecessary construction")]] T get()
 	{
 		return _value.get();
 	}
 
 	struct promise_type
 	{
-		[[nodiscard]] Awaitable get_return_object() noexcept
+		[[nodiscard("unnecessary construction")]] Awaitable get_return_object() noexcept
 		{
 			return { _promise.get_future() };
 		}
@@ -105,7 +105,7 @@ public:
 
 	};
 
-	[[nodiscard]] constexpr bool await_ready() const noexcept
+	[[nodiscard("unexpected call")]] constexpr bool await_ready() const noexcept
 	{
 		return true;
 	}
@@ -115,7 +115,7 @@ public:
 		h.resume();
 	}
 
-	[[nodiscard]] T await_resume()
+	[[nodiscard("unnecessary construction")]] T await_resume()
 	{
 		return _value.get();
 	}

doc/fieldparams.md

@@ -21,7 +21,7 @@ The `graphql::service::FieldParams` struct is declared in [GraphQLService.h](../
 ```cpp
 // Pass a common bundle of parameters to all of the generated Object::getField accessors in a
 // SelectionSet
-struct [[nodiscard]] SelectionSetParams
+struct [[nodiscard("unnecessary construction")]] SelectionSetParams
 {
 	// Context for this selection set.
 	const ResolverContext resolverContext;
@@ -46,7 +46,7 @@ struct [[nodiscard]] SelectionSetParams
 };
 
 // Pass a common bundle of parameters to all of the generated Object::getField accessors.
-struct [[nodiscard]] FieldParams : SelectionSetParams
+struct [[nodiscard("unnecessary construction")]] FieldParams : SelectionSetParams
 {
 	GRAPHQLSERVICE_EXPORT explicit FieldParams(
 		SelectionSetParams&& selectionSetParams, Directives directives);
@@ -64,7 +64,7 @@ The `SelectionSetParams::resolverContext` enum member informs the `getField`
 accessors about what type of operation is being resolved:
 ```cpp
 // Resolvers may be called in multiple different Operation contexts.
-enum class [[nodiscard]] ResolverContext {
+enum class [[nodiscard("unnecessary construction")]] ResolverContext {
 	// Resolving a Query operation.
 	Query,
 
@@ -94,7 +94,7 @@ The `SelectionSetParams::state` member is a reference to the
 // any per-request state that you want to maintain throughout the request (e.g. optimizing or
 // batching backend requests), you can inherit from RequestState and pass it to Request::resolve to
 // correlate the asynchronous/recursive callbacks and accumulate state in it.
-struct [[nodiscard]] RequestState : std::enable_shared_from_this<RequestState>
+struct [[nodiscard("unnecessary construction")]] RequestState : std::enable_shared_from_this<RequestState>
 {
 	virtual ~RequestState() = default;
 };

doc/json.md

@@ -46,7 +46,7 @@ to some other output mechanism, without building a single string buffer for the
 document in memory. For example, you might use this to write directly to a buffered IPC pipe
 or network connection:
 ```cpp
-class [[nodiscard]] Writer final
+class [[nodiscard("unnecessary construction")]] Writer final
 {
 private:
 	struct Concept

doc/responses.md

@@ -10,7 +10,7 @@ represented in GraphQL, as of the
 [October 2021 spec](https://spec.graphql.org/October2021/#sec-Serialization-Format):
 
 ```c++
-enum class [[nodiscard]] Type : std::uint8_t {
+enum class [[nodiscard("unnecessary conversion")]] Type : std::uint8_t {
 	Map,	   // JSON Object
 	List,	   // JSON Array
 	String,	   // JSON String

doc/subscriptions.md

@@ -13,13 +13,13 @@ the subscriptions to those listeners.
 Subscriptions are created by calling the `Request::subscribe` method in
 [GraphQLService.h](../include/graphqlservice/GraphQLService.h):
 ```cpp
-GRAPHQLSERVICE_EXPORT [[nodiscard]] AwaitableSubscribe subscribe(RequestSubscribeParams params);
+GRAPHQLSERVICE_EXPORT [[nodiscard("leaked subscription")]] AwaitableSubscribe subscribe(RequestSubscribeParams params);
 ```
 
 You need to fill in a `RequestSubscribeParams` struct with the subscription event
 callback, the [parsed](./parsing.md) `query` and any other relevant operation parameters:
 ```cpp
-struct [[nodiscard]] RequestSubscribeParams
+struct [[nodiscard("unnecessary construction")]] RequestSubscribeParams
 {
 	// Callback which receives the event data.
 	SubscriptionCallback callback;
@@ -64,13 +64,13 @@ The `internal::Awaitable<T>` template is described in [awaitable.md](./awaitable
 Subscriptions are removed by calling the `Request::unsubscribe` method in
 [GraphQLService.h](../include/graphqlservice/GraphQLService.h):
 ```cpp
-GRAPHQLSERVICE_EXPORT [[nodiscard]] AwaitableUnsubscribe unsubscribe(RequestUnsubscribeParams params);
+GRAPHQLSERVICE_EXPORT [[nodiscard("potentially leaked subscription")]] AwaitableUnsubscribe unsubscribe(RequestUnsubscribeParams params);
 ```
 
 You need to fill in a `RequestUnsubscribeParams` struct with the `SubscriptionKey`
 returned by `Request::subscribe` in `AwaitableSubscribe`:
 ```cpp
-struct [[nodiscard]] RequestUnsubscribeParams
+struct [[nodiscard("unnecessary construction")]] RequestUnsubscribeParams
 {
 	// Key returned by a previous call to subscribe.
 	SubscriptionKey key;
@@ -117,7 +117,7 @@ The `Request::deliver` method determines which subscriptions should receive
 an event based on several factors, which makes the `RequestDeliverParams` struct
 more complicated:
 ```cpp
-struct [[nodiscard]] RequestDeliverParams
+struct [[nodiscard("unnecessary construction")]] RequestDeliverParams
 {
 	// Deliver to subscriptions on this field.
 	std::string_view field;
@@ -155,7 +155,7 @@ using SubscriptionArguments = std::map<std::string_view, response::Value>;
 using SubscriptionArgumentFilterCallback = std::function<bool(response::MapType::const_reference)>;
 using SubscriptionDirectiveFilterCallback = std::function<bool(Directives::const_reference)>;
 
-struct [[nodiscard]] SubscriptionFilter
+struct [[nodiscard("unnecessary construction")]] SubscriptionFilter
 {
 	// Optional field argument filter, which can either be a set of required arguments, or a
 	// callback which returns true if the arguments match custom criteria.
@@ -186,6 +186,6 @@ that, there's a public `Request::findOperationDefinition` method which returns
 the operation type as a `std::string_view` along with a pointer to the AST node
 for the selected operation in the parsed query:
 ```cpp
-GRAPHQLSERVICE_EXPORT [[nodiscard]] std::pair<std::string_view, const peg::ast_node*>
+GRAPHQLSERVICE_EXPORT [[nodiscard("unnecessary call")]] std::pair<std::string_view, const peg::ast_node*>
 findOperationDefinition(peg::ast& query, std::string_view operationName) const;
 ```

include/ClientGenerator.h

@@ -11,53 +11,57 @@
 
 namespace graphql::generator::client {
 
-struct [[nodiscard]] GeneratorPaths
+struct [[nodiscard("unnecessary construction")]] GeneratorPaths
 {
 	const std::string headerPath;
 	const std::string sourcePath;
 };
 
-struct [[nodiscard]] GeneratorOptions
+struct [[nodiscard("unnecessary construction")]] GeneratorOptions
 {
 	const GeneratorPaths paths;
 	const bool verbose = false;
 };
 
-class [[nodiscard]] Generator
+class [[nodiscard("unnecessary construction")]] Generator
 {
 public:
 	// Initialize the generator with the introspection client or a custom GraphQL client.
-	explicit Generator(
-		SchemaOptions&& schemaOptions, RequestOptions&& requestOptions, GeneratorOptions&& options);
+	explicit Generator(SchemaOptions && schemaOptions,
+		RequestOptions && requestOptions,
+		GeneratorOptions && options);
 
 	// Run the generator and return a list of filenames that were output.
-	[[nodiscard]] std::vector<std::string> Build() const noexcept;
+	[[nodiscard("unnecessary memory copy")]] std::vector<std::string> Build() const noexcept;
 
 private:
-	[[nodiscard]] std::string getHeaderDir() const noexcept;
-	[[nodiscard]] std::string getSourceDir() const noexcept;
-	[[nodiscard]] std::string getHeaderPath() const noexcept;
-	[[nodiscard]] std::string getSourcePath() const noexcept;
-	[[nodiscard]] const std::string& getClientNamespace() const noexcept;
-	[[nodiscard]] const std::string& getOperationNamespace(
+	[[nodiscard("unnecessary memory copy")]] std::string getHeaderDir() const noexcept;
+	[[nodiscard("unnecessary memory copy")]] std::string getSourceDir() const noexcept;
+	[[nodiscard("unnecessary memory copy")]] std::string getHeaderPath() const noexcept;
+	[[nodiscard("unnecessary memory copy")]] std::string getSourcePath() const noexcept;
+	[[nodiscard("unnecessary call")]] const std::string& getClientNamespace() const noexcept;
+	[[nodiscard("unnecessary call")]] const std::string& getOperationNamespace(
 		const Operation& operation) const noexcept;
-	[[nodiscard]] std::string getResponseFieldCppType(
-		const ResponseField& responseField, std::string_view currentScope = {}) const noexcept;
+	[[nodiscard("unnecessary memory copy")]] std::string getResponseFieldCppType(
+		const ResponseField& responseField,
+		std::string_view currentScope = {}) const noexcept;
 
-	[[nodiscard]] bool outputHeader() const noexcept;
-	void outputRequestComment(std::ostream& headerFile) const noexcept;
-	void outputGetRequestDeclaration(std::ostream& headerFile) const noexcept;
-	void outputGetOperationNameDeclaration(std::ostream& headerFile) const noexcept;
-	[[nodiscard]] bool outputResponseFieldType(std::ostream& headerFile,
-		const ResponseField& responseField, size_t indent = 0) const noexcept;
+	[[nodiscard("unnecessary call")]] bool outputHeader() const noexcept;
+	void outputRequestComment(std::ostream & headerFile) const noexcept;
+	void outputGetRequestDeclaration(std::ostream & headerFile) const noexcept;
+	void outputGetOperationNameDeclaration(std::ostream & headerFile) const noexcept;
+	[[nodiscard("unnecessary call")]] bool outputResponseFieldType(std::ostream & headerFile,
+		const ResponseField& responseField,
+		size_t indent = 0) const noexcept;
 
-	[[nodiscard]] bool outputSource() const noexcept;
-	void outputGetRequestImplementation(std::ostream& sourceFile) const noexcept;
-	void outputGetOperationNameImplementation(
-		std::ostream& sourceFile, const Operation& operation) const noexcept;
-	bool outputModifiedResponseImplementation(std::ostream& sourceFile,
-		const std::string& outerScope, const ResponseField& responseField) const noexcept;
-	[[nodiscard]] static std::string getTypeModifierList(
+	[[nodiscard("unnecessary call")]] bool outputSource() const noexcept;
+	void outputGetRequestImplementation(std::ostream & sourceFile) const noexcept;
+	void outputGetOperationNameImplementation(std::ostream & sourceFile, const Operation& operation)
+		const noexcept;
+	bool outputModifiedResponseImplementation(std::ostream & sourceFile,
+		const std::string& outerScope,
+		const ResponseField& responseField) const noexcept;
+	[[nodiscard("unnecessary memory copy")]] static std::string getTypeModifierList(
 		const TypeModifierStack& modifiers) noexcept;
 
 	const SchemaLoader _schemaLoader;

include/GeneratorLoader.h

@@ -11,7 +11,7 @@
 namespace graphql::generator {
 
 // Types that we understand and use to generate the skeleton of a service.
-enum class [[nodiscard]] SchemaType {
+enum class [[nodiscard("unnecessary conversion")]] SchemaType {
 	Scalar,
 	Enum,
 	Input,
@@ -30,10 +30,11 @@ using TypeModifierStack = std::vector<service::TypeModifier>;
 
 // Recursively visit a Type node until we reach a NamedType and we've
 // taken stock of all of the modifier wrappers.
-class [[nodiscard]] TypeVisitor
+class [[nodiscard("unnecessary construction")]] TypeVisitor
 {
 public:
-	[[nodiscard]] std::pair<std::string_view, TypeModifierStack> getType();
+	[[nodiscard("unnecessary construction")]] std::pair<std::string_view, TypeModifierStack>
+	getType();
 
 	void visit(const peg::ast_node& typeName);
 
@@ -49,10 +50,10 @@ class [[nodiscard]] TypeVisitor
 
 // Recursively visit a Value node representing the default value on an input field
 // and build a JSON representation of the hardcoded value.
-class [[nodiscard]] DefaultValueVisitor
+class [[nodiscard("unnecessary construction")]] DefaultValueVisitor
 {
 public:
-	[[nodiscard]] response::Value getValue();
+	[[nodiscard("unnecessary construction")]] response::Value getValue();
 
 	void visit(const peg::ast_node& value);
 

include/GeneratorUtil.h

@@ -13,10 +13,10 @@
 namespace graphql::generator {
 
 // RAII object to help with emitting matching include guard begin and end statements
-class [[nodiscard]] IncludeGuardScope
+class [[nodiscard("unnecessary construction")]] IncludeGuardScope
 {
 public:
-	explicit IncludeGuardScope(std::ostream& outputFile, std::string_view headerFileName) noexcept;
+	explicit IncludeGuardScope(std::ostream & outputFile, std::string_view headerFileName) noexcept;
 	~IncludeGuardScope() noexcept;
 
 private:
@@ -25,12 +25,13 @@ class [[nodiscard]] IncludeGuardScope
 };
 
 // RAII object to help with emitting matching namespace begin and end statements
-class [[nodiscard]] NamespaceScope
+class [[nodiscard("unnecessary construction")]] NamespaceScope
 {
 public:
-	explicit NamespaceScope(
-		std::ostream& outputFile, std::string_view cppNamespace, bool deferred = false) noexcept;
-	NamespaceScope(NamespaceScope&& other) noexcept;
+	explicit NamespaceScope(std::ostream & outputFile,
+		std::string_view cppNamespace,
+		bool deferred = false) noexcept;
+	NamespaceScope(NamespaceScope && other) noexcept;
 	~NamespaceScope() noexcept;
 
 	bool enter() noexcept;
@@ -44,10 +45,10 @@ class [[nodiscard]] NamespaceScope
 
 // Keep track of whether we want to add a blank separator line once some additional content is about
 // to be output.
-class [[nodiscard]] PendingBlankLine
+class [[nodiscard("unnecessary construction")]] PendingBlankLine
 {
 public:
-	explicit PendingBlankLine(std::ostream& outputFile) noexcept;
+	explicit PendingBlankLine(std::ostream & outputFile) noexcept;
 
 	void add() noexcept;
 	bool reset() noexcept;